Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix crash after receiving a KickedOff message #1521

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Attempt to fix crash after receiving a KickedOff message #1521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a5af3f2
Assert that socket is open before trying to operate on it
dgelessus Nov 3, 2023
9c979fc
Lock more consistently during socket operations
dgelessus Nov 3, 2023
8a73bfc
Fix crash after receiving an Auth2Cli_KickedOff message
dgelessus Nov 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions Sources/Plasma/NucleusLib/pnAsyncCoreExe/pnAceSocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,19 +184,24 @@ static void SocketStartAsyncRead(AsyncSocket sock)
hsLockGuard(sock->fCritsect);
uint8_t* start = sock->fBuffer + sock->fBytesLeft;
size_t count = sizeof(sock->fBuffer) - sock->fBytesLeft;
hsAssert(sock->fSock.is_open(), "Read from a closed socket");
sock->fSock.async_read_some(asio::buffer(start, count),
[sock](const asio::error_code& err, size_t bytes) {
if (err) {
bool isEOFError = (err.category() == asio::error::get_misc_category() && err.value() == asio::error::eof);
bool isAbortedError = (err.category() == asio::error::get_system_category() && err.value() == asio::error::operation_aborted);
if (isEOFError || isAbortedError) {
if (sock->fNotifyProc) {
FAsyncNotifySocketProc notifyProc = nullptr;
{
hsLockGuard(sock->fCritsect);
notifyProc = sock->fNotifyProc;
sock->fNotifyProc = nullptr;
}
if (notifyProc) {
// We have to be extremely careful from this point because
// sockets can be deleted during the notification callback.
// After this call, the application becomes responsible for
// calling AsyncSocketDelete at some later point in time.
FAsyncNotifySocketProc notifyProc = sock->fNotifyProc;
sock->fNotifyProc = nullptr;
notifyProc((AsyncSocket)sock, kNotifySocketDisconnect, nullptr, &sock->fUserState);
}
} else {
Expand All @@ -208,6 +213,7 @@ static void SocketStartAsyncRead(AsyncSocket sock)
if (!bytes)
return;

hsLockGuard(sock->fCritsect);
sock->fBytesLeft += bytes;

AsyncNotifySocketRead notifyRead;
Expand Down Expand Up @@ -273,9 +279,11 @@ static bool SocketInitConnect(ConnectOperation& op)
{
// This steals ownership of op->fSock
auto sock = std::make_unique<AsyncSocketStruct>(op);
hsLockGuard(sock->fCritsect);

sock->initTimeMs = hsTimer::GetMilliSeconds<unsigned>();

hsAssert(sock->fSock.is_open(), "Socket closed during connect?");
asio::error_code err;
sock->fSock.non_blocking(true, err);
if (err)
Expand Down Expand Up @@ -341,6 +349,7 @@ void AsyncSocketConnect(AsyncCancelId* cancelId, const plNetAddress& netAddr,
tcp::endpoint remoteAddr(asio::ip::address_v4(netAddr.GetHostBytes()),
netAddr.GetPort());
op->fSock.async_connect(remoteAddr, [op](const asio::error_code& connError) {
hsLockGuard(s_connectCrit);
// This operation is no longer a candidate for cleanup or cancellation
// by the time we get a callback from async_connect
bool success;
Expand All @@ -360,10 +369,7 @@ void AsyncSocketConnect(AsyncCancelId* cancelId, const plNetAddress& netAddr,
op->fNotifyProc(nullptr, kNotifySocketConnectFailed, &failed, nullptr);
}

{
hsLockGuard(s_connectCrit);
s_connectList.erase(op);
}
s_connectList.erase(op);

PerfSubCounter(kAsyncPerfSocketConnAttemptsOutCurr, 1);
});
Expand Down Expand Up @@ -402,6 +408,7 @@ static void HardCloseSocket(AsyncSocket conn)

void AsyncSocketDisconnect(AsyncSocket conn, bool hardClose)
{
hsLockGuard(conn->fCritsect);
if (!conn->fSock.is_open())
return;

Expand All @@ -417,7 +424,7 @@ void AsyncSocketDisconnect(AsyncSocket conn, bool hardClose)

static bool SocketQueueAsyncWrite(AsyncSocket conn, const void* data, size_t bytes)
{
hsLockGuard(s_connectCrit);
hsLockGuard(conn->fCritsect);

// check for data backlog
if (!conn->fWriteOps.empty()) {
Expand Down Expand Up @@ -478,8 +485,9 @@ static bool SocketQueueAsyncWrite(AsyncSocket conn, const void* data, size_t byt
op->fNotify.bytesCommitted = op->fNotify.bytes;
}
}
hsAssert(conn->fSock.is_open(), "Write to a closed socket");
async_write(conn->fSock, allWrites, [conn](const asio::error_code& err, size_t bytes) {
hsLockGuard(s_connectCrit);
hsLockGuard(conn->fCritsect);
while (bytes != 0) {
hsAssert(conn->fWriteOps.size() > 0, "buffer mismatch");
WriteOperation* op = conn->fWriteOps.front();
Expand Down Expand Up @@ -508,6 +516,7 @@ bool AsyncSocketSend(AsyncSocket conn, const void* data, size_t bytes)
// data right away, we do so in order to save extra allocations for the
// write buffer. Otherwise, we must queue it for an async write below.
if (conn->fWriteOps.empty()) {
hsAssert(conn->fSock.is_open(), "Write to a closed socket");
asio::error_code err;
size_t bytesSent = conn->fSock.write_some(asio::buffer(data, bytes), err);
if (!err) {
Expand Down
1 change: 0 additions & 1 deletion Sources/Plasma/PubUtilLib/plNetGameLib/Private/plNglAuth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,6 @@ bool RecvMsg<Auth2Cli_KickedOff>(const uint8_t buffer[], unsigned bytes, void* p
const Auth2Cli_KickedOff & msg = *(const Auth2Cli_KickedOff *)buffer;

ReportNetError(kNetProtocolCli2Auth, msg.reason);
NetCliAuthDisconnect();

return true;
}
Expand Down