[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:
Depends
Related
Sprint: Platforms 2018-05-07
Participants:
Linked BF Score: 0

 Description   

Affected files:

  • util/net/listen.h
  • util/net/listen.cpp

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:

  1. Listener::setupSockets() called
    1. some number of sockets are created
    2. sockets are added to Listener::_socks
    3. sockets are added to ListeningSockets::add()
  2. Listener::initAndListen() called
    1. The Listener begins looping over the sockets it created. It does this until inShutdown()
  3. ListeningSockets::closeAll() called
    1. the mutex doesn't do anything because Listener has the same set of sockets in it's own private vector
    2. Sockets are all closed immediately
  4. Listener::initAndListen() asserts with an invalid socket from either select() or WSAEventSelect()
    1. one of the sockets Listener is iterating over has been closed, and is no longer valid

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.

Generated at Thu Feb 08 03:29:37 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.