erpc icon indicating copy to clipboard operation
erpc copied to clipboard

Server with TCP Transport handling multiple connections

Open mentaal opened this issue 6 years ago • 7 comments

Hi, I see that in tcp_transport.cpp, if the connection is closed by the client, TCPTransport::close() brings down the server thread by setting m_runServer = false; Please correct me if I'm wrong but it seems to me that this means that for a subsequent client connection, the server and transport need to be reinitialized again. This also looks like it would incur the overhead of having to create a new thread. I was just wondering if you could comment on why this approach was taken?

mentaal avatar Jun 13 '18 09:06 mentaal

Hi @mentaal, i am sorry i was not working on this transport. But as i see it you can reuse only "open" and/or "configure" functions again. You don't need new server or thread. This way you can decide what happen with server when client will close connection.

Hadatko avatar Jun 13 '18 09:06 Hadatko

Hi @Hadatko, No problem. I was just curious. Indeed, TCPTransport::open looks like it is required to be rerun but note that it does call m_serverThread.start(this); which creates a new thread under the hood. This is because when ::close() is called, m_runServer is set to false, which breaks out of serverThread's while loop which closes the server's socket the the server thread completes its execution.

I am thinking about refactoring this code a bit for my project to allow the server to continue waiting for new connections without having to reinitialize the server. I am also a bit skeptical about having accept() and read() run concurrently without providing more infrastructure for concurrent client support in general. (Though I am no expert in network programming - in fact I have very little experience in this domain so take this with a grain of salt!) For example, what should happen if a new connection is accepted whilst read is still blocking, waiting for more data on the old socket? This stackoverflow comment is worth reading on that thought: https://stackoverflow.com/questions/3589723/can-a-socket-be-closed-from-another-thread-when-a-send-recv-on-the-same-socket#27790293

I have more thinking to do on the topic. Any feedback you have on it is most welcome.

mentaal avatar Jun 13 '18 10:06 mentaal

Hi @mentaal, i forgot that start function is calling create new thread function. Thanks for you minds. Sounds reasonable. Currently i don't have much time to investigate this things. But you can made your changes and give us pull request. But for pull requests this size i will need sign contribution agreement from you if it is ok.

Hadatko avatar Jun 13 '18 10:06 Hadatko

Of course, that is no problem at all. I could be a while with it as I have to develop it first.

mentaal avatar Jun 13 '18 10:06 mentaal

Here is Contributor agreement. Contributor Agreement.docx

Hadatko avatar Jun 13 '18 11:06 Hadatko

Here's a link to a branch implementing a tcp server without threading. Before adding it as a pull request, tell me what you think: https://github.com/mentaal/erpc/tree/tcp_no_threading

mentaal avatar Jun 22 '18 08:06 mentaal

Hi @mentaal , although i like your idea, do you think it is good idea to keep two solutions of one transport layer?

Hadatko avatar May 14 '19 13:05 Hadatko