Feature request: make telnet-proxy support multiple connections

[marado]

Using telnet-proxy for debug purposes is great, but it would be quite more useful if it would support multiple connections.

[seanmiddleditch]

I'll accept a pull request for this if anyone has one. :)

I'll note that the debug output for such a change must make it easy to identify which connection the output's coming from.

[Beliriel]

I wrote a multi-connection proxy for this library, however I used pthreads and had to modify the cmake to incorporate the pthread library. As such it will likely only work for Linux distributions (because I have no idea how to compile on or for windows) and it now has a dependency. The debug output hasn't been modified so far and you can't differentiate between different connections on the proxy (yet).

The proxy uses different threads for connections as my first single-thread try crashed pretty hard. I also didn't have to modify the polling function a lot with threads.

Is there interest in it?

[Beliriel]

I want to add that a proxy that can handle multiple connections is probably not as portable as this project intends to be. Implementing a single threaded proxy like that will make it ugly and inefficient (because the poll- and accept-function block the thread and the select-function technically too) while a multithreaded one will not be as portable as multithreading is handled differently on different platforms.

Nonetheless for Linux I made this: https://github.com/Beliriel/libtelnet
but I will not make a pull request because it is not portable and the debug output still hasn't been modified to make it easy to identify connections.

[seanmiddleditch]

@Beliriel - perhaps an option would be to add a new multi-proxy binary that is optionally built if find_package(Threads) success? Should make it easy to make things portable

It should be totally possible to multiplex multiple connections in a single thread, that's one of the primary use cases I originally wrote this library for (multiplayer text games / MUDs).

CI should keep us honest re portability if you make a PR.

[Beliriel]

I went back and looked at it. Has been a while. I just remembered that I ran into issues. But not because of the blocking (well it did, but that was kinda my own fault) but because of memory management. Maybe I just thought too big, but on a big server you can have a lot of traffic, which is to say, that a very busy connection will hog the thread. For testing purposes it might be okay but for professional (100 - 10000 connections) settings that could pose an issue, when you can have a lot of busy connections. The far bigger problem though was that poll() uses a simple array to monitor all the file descriptors, which in our case are sockets. The problem here is that in case of a status change of a socket (read file descriptor) you have to completely block off any operations done on the sockets, i.e. poll() shouldn't be called, manage the array and then resume use again. This means that any new incoming connection will interrupt reading and writing to all sockets. I mean you still can read and write to the sockets but it's better if you don't as it could lead to unstable behaviour. Even worse, closing a connection in the middle of the array now means you have to block too and then reallocate every connection socket that came after the closing one (unless you want to impose a hardcoded limit on the number of connections).

I have to admit it's a solution to a problem I could never emulate on my home computer so I'll be revisiting a single threaded approach. To me multiplexing over multiple threads sounds just easier than managing memory access of the file descriptor array as I only have to maintain one doubly linked list across all threads and one condition variable to track new incoming connections. This means any closing or opening of sockets (and subsequent manipulation of the linked list) doesn't impede on the read/write operations of other connections since they get handled in different threads with their own specific sockets.

Still even with polling in a single thread you'll need 2-3 threads: main thread to receive signals and provide stdin, accept-thread that spins off new connections and a polling thread, that handles communcation between the parties.

In my personal opinion, it's cleaner to handle every connection pair in a separate thread. You'd have to iterate over every socket if poll() returns events in the queue in a single thread even if it's just one connection talking. That's a lot of "dead" processing you have to do.

[seanmiddleditch]

The far bigger problem though was that poll() uses a simple array to monitor all the file descriptors, which in our case are sockets. The problem here is that in case of a status change of a socket (read file descriptor) you have to completely block off any operations done on the sockets, i.e. poll() shouldn't be called, manage the array and then resume use again. This means that any new incoming connection will interrupt reading and writing to all sockets.

Not sure what you're saying here. There should be no need to do any blocking calls or interrupt any read/write operations, and managing the poll array is pretty light-weight with the correct data structures. The kernel-side of poll isn't the most efficient, but that's what epoll/kqueue/IOCP/etc. are for. For the use case of telnet-proxy, which is mostly a debugging tool in my intended use case, I'd be a lot less concerned about theoretical efficiency and more concerned about correctness, simplicity, and portability. (And on that front, I think you might have to use select instead of poll anyway)

In my personal opinion, it's cleaner to handle every connection pair in a separate thread.

It adds the complication of needing to ensure that the debug output stream is synchronized and not interleaved, though, which does impose some complication and possibly a lot of locking/unlocking (which is likely to swamp the cost vs iterating the array for poll).