DEV9: Fix race condition in UDP sockets

This commit is contained in:
TheLastRar 2025-04-12 16:32:50 +01:00
parent 68cdf08968
commit e4eea70ac0
2 changed files with 34 additions and 10 deletions

View File

@ -16,5 +16,6 @@ namespace Sessions
} }
virtual bool WillRecive(PacketReader::IP::IP_Address parDestIP) = 0; virtual bool WillRecive(PacketReader::IP::IP_Address parDestIP) = 0;
virtual void ForceClose() { RaiseEventConnectionClosed(); };
}; };
} // namespace Sessions } // namespace Sessions

View File

@ -84,8 +84,28 @@ namespace Sessions
if (!success) if (!success)
{ {
RaiseEventConnectionClosed(); // See Reset() for why we copy the vector.
return std::nullopt; std::vector<UDP_BaseSession*> connectionsCopy;
{
std::lock_guard numberlock(connectionSentry);
open.store(false);
connectionsCopy = connections;
}
if (connectionsCopy.size() == 0)
{
// Can close immediately.
RaiseEventConnectionClosed();
return std::nullopt;
}
else
{
// Need to wait for child connections to close.
for (size_t i = 0; i < connectionsCopy.size(); i++)
connectionsCopy[i]->ForceClose();
return std::nullopt;
}
} }
else if (ret.has_value()) else if (ret.has_value())
{ {
@ -112,10 +132,12 @@ namespace Sessions
void UDP_FixedPort::Reset() void UDP_FixedPort::Reset()
{ {
// Reseting a session may cause that session to close itself, /*
// when that happens, the connections vector gets modified via our close handler. * Reseting a session may cause that session to close itself,
// Duplicate the vector to avoid iterating over a modified collection, * when that happens, the connections vector gets modified via our close handler.
// this also avoids the issue of recursive locking when our close handler takes a lock. * Duplicate the vector to avoid iterating over a modified collection,
* this also avoids the issue of recursive locking when our close handler takes a lock.
*/
std::vector<UDP_BaseSession*> connectionsCopy; std::vector<UDP_BaseSession*> connectionsCopy;
{ {
std::lock_guard numberlock(connectionSentry); std::lock_guard numberlock(connectionSentry);
@ -128,16 +150,15 @@ namespace Sessions
UDP_Session* UDP_FixedPort::NewClientSession(ConnectionKey parNewKey, bool parIsBrodcast, bool parIsMulticast) UDP_Session* UDP_FixedPort::NewClientSession(ConnectionKey parNewKey, bool parIsBrodcast, bool parIsMulticast)
{ {
// Lock the whole function so we can't race between the open check and creating the session
std::lock_guard numberlock(connectionSentry);
if (!open.load()) if (!open.load())
return nullptr; return nullptr;
UDP_Session* s = new UDP_Session(parNewKey, adapterIP, parIsBrodcast, parIsMulticast, client); UDP_Session* s = new UDP_Session(parNewKey, adapterIP, parIsBrodcast, parIsMulticast, client);
s->AddConnectionClosedHandler([&](BaseSession* session) { HandleChildConnectionClosed(session); }); s->AddConnectionClosedHandler([&](BaseSession* session) { HandleChildConnectionClosed(session); });
{ connections.push_back(s);
std::lock_guard numberlock(connectionSentry);
connections.push_back(s);
}
return s; return s;
} }
@ -159,6 +180,8 @@ namespace Sessions
UDP_FixedPort::~UDP_FixedPort() UDP_FixedPort::~UDP_FixedPort()
{ {
DevCon.WriteLn("DEV9: Socket: UDPFixedPort %d had %d child connections", port, connections.size());
open.store(false); open.store(false);
if (client != INVALID_SOCKET) if (client != INVALID_SOCKET)
{ {