[SERVER-12789] A race between sockets owned by Listener & ListeningSockets Created: 19/Feb/14 Updated: 01/May/18 Resolved: 01/May/18 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Networking |
| Affects Version/s: | 2.5.5 |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Trivial - P5 |
| Reporter: | Mira Carey | Assignee: | DO NOT USE - Backlog - Platform Team |
| Resolution: | Done | Votes: | 1 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Sprint: | Platforms 2018-05-07 | ||||||||
| Participants: | |||||||||
| Linked BF Score: | 0 | ||||||||
| Description |
|
Affected files:
The sockets created by Listener::setupSockets() and stored in Listener::_socks are also passed to ListeningSockets::add(). This creates a point of dual ownership, where ListeningSockets::closeAll() is responsible for closing sockets that Listener::initAndListen() loops over until inShutdown(). More specifically the race comes from:
I'm not entirely clear on whether the server manages this shutdown cleanly (by putting the Listener into inShutdown() before calling closeAll()), or if calling dbexit() leads to an _exit() fast enough to skip by the race. But either way it's awkward in the extreme to make another, ostensibly thread safe, class responsible for resources that Listener owns and continuously works on. Ideally Listener would either be responsible for it's own sockets, or ListeningSockets would have some kind of api for checking sockets out for a duration to allow for actual thread safe processing. |
| Comments |
| Comment by Andrew Morrow (Inactive) [ 01/May/18 ] |
|
The referenced code no longer exists on master, closing as gone away. |
| Comment by Mira Carey [ 15/Sep/17 ] |
|
Marking this trivial because the legacy transport layer is no longer the default for 3.6 |
| Comment by Samantha Ritter (Inactive) [ 13/Dec/16 ] |
|
A related race exists in the TransportLayer, with connections that have been accepted already. The TransportLayer exposes an endAllSessions() method that closes the underlying sockets for those sessions. We should never allow sockets to be closed from other threads. |