[ https://issues.apache.org/jira/browse/THRIFT-2441?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14392880#comment-14392880 ]
Ben Craig edited comment on THRIFT-2441 at 4/2/15 4:00 PM: ----------------------------------------------------------- I think we're converging on a great answer. Thanks for being patient with me :). I'll pick apart this reply now, and offer some suggestions on the details. "It would also be necessary to have the death_listener be a third socket pair that is independent of the interrupt socket pair, where the server socket owns the sending side and the server socket + client sockets communally own the receiving side of the death_listener, and the clients do not attempt to read off it to clear the condition when it is set. " I think this is a reasonable middle ground. A server socket is "fat" in that it creates five file descriptors, but each connected client only makes one additional file descriptor (as opposed to the three additional that prior proposals required). "Per Randy's provided use case we need to maintain that functionality, so it would be necessary to add a stop() method that first sets a boolean stop flag, and then calls interrupt in much the same way." So here are the two big source compatibility cases that we need to watch out for... Pre-existing custom transport used with TThreadedServer TServerSocket and TSocket used with a custom server Here's an approach that might fix both... 1. In TServerTransport, add a virtual method "interruptServerAndChildren()", and give it a default implementation that just forwards directly to "interrupt()" a. The default implementation is important so that we don't break custom transports 2. Implement "interruptServerAndChildren()" in TServerSocket such that it calls "interrupt()", then writes to the extra socket pair that children are listening to. a. This leaves the semantics of "interrupt()" untouched, so that old servers still do the same thing 3. Retrofit TThreadedServer, TThreadPoolServer, and TSimpleServer so that they call "interruptServerAndChildren()" instead of "interrupt()". a. TNonBlockingServer is its own special beast. Messing with that seems like it is out of the scope of these changes. "For that reason I would make the death_listener a shared pointer with a custom deletion method..." Yep, sounds great. Nice catch on lifetime management of this particular resource. was (Author: ben.craig): I think we're converging on a great answer. Thanks for being patient with me :). I'll pick apart this reply now, and offer some suggestions on the details. "It would also be necessary to have the death_listener be a third socket pair that is independent of the interrupt socket pair, where the server socket owns the sending side and the server socket + client sockets communally own the receiving side of the death_listener, and the clients do not attempt to read off it to clear the condition when it is set. " I think this is a reasonable middle ground. A server socket is "fat" in that it creates five file descriptors, but each connected client only makes one additional file descriptor (as opposed to the three additional that prior proposals required). "Per Randy's provided use case we need to maintain that functionality, so it would be necessary to add a stop() method that first sets a boolean stop flag, and then calls interrupt in much the same way." So here are the two big source compatibility cases that we need to watch out for... Pre-existing custom transport used with TThreadedServer TServerSocket and TSocket used with a custom server Here's an approach that might fix both... 1. In TServerTransport, add a virtual method "interruptServerAndChildren()", and give it a default implementation that just forwards directly to "interrupt()" a. The default implementation is important so that we don't break custom transports 2. Implement "interruptServerAndChildren()" in TServerSocket such that it calls "interrupt()", then writes to the extra socket pair that children are listening to. a. This leaves the semantics of "interrupt()" untouched, so that old servers still do the same thing 3. Retrofit TThreadedServer, TThreadPoolServer, and TSimpleServer so that they call "interruptServerAndChildren()" instead of "interrupt()". a. TNonBlockingServer is its own special beast. Messing with that seems like it is out of the scope of these changes. "For that reason I would make the death_listener a shared pointer with a custom deletion method..." Yep, sounds great. Nice catch on lifetime management of this particular resource. > Cannot shutdown TThreadedServer when clients are still connected > ---------------------------------------------------------------- > > Key: THRIFT-2441 > URL: https://issues.apache.org/jira/browse/THRIFT-2441 > Project: Thrift > Issue Type: Bug > Components: C++ - Library > Affects Versions: 0.9.1 > Reporter: Chris Stylianou > Assignee: Ben Craig > > When calling stop() on the TThreadedServer no interrupts are sent to the > client threads. This means the stop() call blocks on tasksMonitor.wait() > until all client naturally disconnect. > How can we tell the client thread connections to close/exit during the > TThreadedServer::stop() call? -- This message was sent by Atlassian JIRA (v6.3.4#6332)