Skip to content

Commit

Permalink
Native Posix: Use ::shutdown() instead of ::close()
Browse files Browse the repository at this point in the history
  • Loading branch information
0blu committed Nov 1, 2024
1 parent ff63608 commit 948f066
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 24 deletions.
4 changes: 1 addition & 3 deletions src/shared/IO/Networking/AsyncSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,14 @@ IO::Networking::AsyncSocket::~AsyncSocket()
return; // Ignore destructor

sLog.Out(LOG_NETWORK, LOG_LVL_DEBUG, "[%s] Destructor called ~AsyncSocket: No references left", GetRemoteIpString().c_str());
CloseSocket();
m_descriptor.CloseSocket(); // <-- This will actually close the socket and release the file descriptor to the kernel

//#ifdef DEBUG
// Logic behind these checks:
// If the destructor is called, there should be no more std::shared_ptr<> references to this object
// Every Read(...) or Write(...) should use `shared_from_this()` if this is not the case, one of the following checks will fail
MANGOS_ASSERT(!(state & SocketStateFlags::CONTEXT_PRESENT));
MANGOS_ASSERT(!(state & SocketStateFlags::WRITE_PRESENT));
MANGOS_ASSERT(!(state & SocketStateFlags::READ_PRESENT));
//#endif // _DEBUG
}

bool IO::Networking::AsyncSocket::IsClosing() const
Expand Down
1 change: 1 addition & 0 deletions src/shared/IO/Networking/AsyncSocketAcceptor_posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ void IO::Networking::AsyncSocketAcceptor::OnNewClientToAcceptAvailable()
if (err)
{
sLog.Out(LOG_NETWORK, LOG_LVL_ERROR, "OnNewClientToAcceptAvailable -> ::IO::Utils::SetFdStatusFlag(...) Error: %s", err.ToString().c_str());
socketDescriptor.CloseSocket();
return;
}

Expand Down
31 changes: 14 additions & 17 deletions src/shared/IO/Networking/AsyncSocket_posix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,15 @@ void IO::Networking::AsyncSocket::Write(IO::ReadableBuffer const& source, std::f

void IO::Networking::AsyncSocket::CloseSocket()
{
// This function will not actually >close< the socket, since this would release the file descriptor and could cause race conditions,
// if the same descriptor id is reused by another socket.

// set SHUTDOWN_PENDING flag, and check if there was already a previous one
if (m_atomicState.fetch_or(SocketStateFlags::SHUTDOWN_PENDING) & SocketStateFlags::SHUTDOWN_PENDING)
return; // there was already a ::close()
return; // there was already a ::shutdown()

sLog.Out(LOG_NETWORK, LOG_LVL_DEBUG, "[%s] CloseSocket(): Disconnect request", GetRemoteIpString().c_str());
m_descriptor.CloseSocket(); // will silently remove from this socket from the epoll/kqueue set
::shutdown(m_descriptor.GetNativeSocket(), SHUT_RDWR);
}

void IO::Networking::AsyncSocket::PerformNonBlockingRead()
Expand Down Expand Up @@ -394,17 +397,18 @@ void IO::Networking::AsyncSocket::PerformContextSwitch()
if (state & SocketStateFlags::CONTEXT_PENDING_LOAD)
return; // Someone else uses it

if (state & SocketStateFlags::IGNORE_TRANSFERS)
{
m_atomicState.fetch_and(~SocketStateFlags::CONTEXT_PENDING_LOAD);
return; // We are not allowed to react to it
}

MANGOS_ASSERT(state & SocketStateFlags::CONTEXT_PRESENT); // why was this function even called if we have no context?

auto tmpCallback = std::move(m_contextCallback);
m_atomicState.fetch_and(~(SocketStateFlags::CONTEXT_PENDING_LOAD | SocketStateFlags::CONTEXT_PRESENT));

if (state & SocketStateFlags::SHUTDOWN_PENDING)
{
m_atomicState.fetch_and(~SocketStateFlags::CONTEXT_PENDING_LOAD);
tmpCallback(IO::NetworkError(IO::NetworkError::ErrorType::SocketClosed));
return; // The socket was closed, no transfers are allowed
}

MANGOS_DEBUG_ASSERT(tmpCallback);
tmpCallback(IO::NetworkError(IO::NetworkError::ErrorType::NoError));
}
Expand All @@ -421,9 +425,7 @@ void IO::Networking::AsyncSocket::StopPendingTransactionsAndForceClose()
int const pendingTransferMask = SocketStateFlags::WRITE_PENDING_SET |
SocketStateFlags::WRITE_PENDING_LOAD |
SocketStateFlags::READ_PENDING_SET |
SocketStateFlags::READ_PENDING_LOAD |
SocketStateFlags::CONTEXT_PENDING_SET |
SocketStateFlags::CONTEXT_PENDING_LOAD;
SocketStateFlags::READ_PENDING_LOAD;
if (state & pendingTransferMask)
{
while ((state = m_atomicState.load()) & pendingTransferMask)
Expand All @@ -447,12 +449,7 @@ void IO::Networking::AsyncSocket::StopPendingTransactionsAndForceClose()
tmpReadCallback(IO::NetworkError(IO::NetworkError::ErrorType::SocketClosed), 0);
}

if (state & SocketStateFlags::CONTEXT_PRESENT)
{
auto tmpContextCallback = std::move(m_contextCallback);
m_atomicState.fetch_and(~SocketStateFlags::CONTEXT_PRESENT);
tmpContextCallback(IO::NetworkError(IO::NetworkError::ErrorType::SocketClosed));
}
// Note: Don't even think about clearing CONTEXT_PRESENT here, since it's stored as a raw pointer in `m_contextSwitchQueue`
}

void IO::Networking::AsyncSocket::EnterIoContext(std::function<void(IO::NetworkError)> const& callback)
Expand Down
4 changes: 2 additions & 2 deletions src/shared/IO/Networking/AsyncSocket_windows.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,9 @@ void IO::Networking::AsyncSocket::CloseSocket()
{
// set SHUTDOWN_PENDING flag, and check if there was already a previous one
if (m_atomicState.fetch_or(SocketStateFlags::SHUTDOWN_PENDING) & SocketStateFlags::SHUTDOWN_PENDING)
return; // there was already a ::close()
return; // there was already a ::shutdown()

m_descriptor.CloseSocket(); // will interrupt and fail all pending IOCP events and post them to the queue
::shutdown(m_descriptor.GetNativeSocket(), SD_BOTH); // will interrupt and fail all pending IOCP events and post them to the queue
}

/// The callback is invoked in the IO thread
Expand Down
3 changes: 1 addition & 2 deletions src/shared/IO/Networking/SocketDescriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ IO::Networking::SocketDescriptor::~SocketDescriptor()

void IO::Networking::SocketDescriptor::CloseSocket()
{
if (m_isClosed)
return;
MANGOS_ASSERT(!m_isClosed);
m_isClosed = true;
IO::Networking::Internal::CloseSocket(m_nativeSocket);
}

0 comments on commit 948f066

Please sign in to comment.