From b4be224004fb39c3d39507cefa929d6422e7f928 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 11:57:43 +0100 Subject: Core/RA: Fix a possible crash Caused when RASocket::handle_close (event-driven) would delete the underlying object before RASocket::commandFinished callback was executed for that object. Dereferencing freed pointers is bad. --- src/server/worldserver/RemoteAccess/RASocket.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src/server/worldserver') diff --git a/src/server/worldserver/RemoteAccess/RASocket.cpp b/src/server/worldserver/RemoteAccess/RASocket.cpp index ee05e83ad4d..3e9c12b32d8 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.cpp +++ b/src/server/worldserver/RemoteAccess/RASocket.cpp @@ -59,7 +59,6 @@ int RASocket::handle_close(ACE_HANDLE, ACE_Reactor_Mask) sLog->outInfo(LOG_FILTER_REMOTECOMMAND, "Closing connection"); peer().close_reader(); wait(); - destroy(); return 0; } @@ -412,10 +411,12 @@ void RASocket::commandFinished(void* callbackArg, bool /*success*/) // the message is 0 size control message to tell that command output is finished // hence we don't put timeout, because it shouldn't increase queue size and shouldn't block - if (socket->putq(mb) == -1) - { + if (socket->peer().get_handle() == ACE_INVALID_HANDLE // this can happen if this code is triggered when handle_close has already called peer().close_writer() + || socket->putq(mb->duplicate()) == -1) // getting here is bad, command can't be marked as complete sLog->outDebug(LOG_FILTER_REMOTECOMMAND, "Failed to enqueue command end message. Error is %s", ACE_OS::strerror(errno)); - mb->release(); - } + + mb->release(); + socket->destroy(); // deletes the object + socket = NULL; } -- cgit v1.2.3 From 203cf7cbf0d860dcc12ab625226b1c7567854356 Mon Sep 17 00:00:00 2001 From: Machiavelli Date: Thu, 14 Feb 2013 13:30:51 +0100 Subject: Core/RA: Addition to previous crashfix To make sure it also works for sessions that use more than 1 command before closing. --- src/server/worldserver/RemoteAccess/RASocket.cpp | 16 ++++++++++++---- src/server/worldserver/RemoteAccess/RASocket.h | 1 + 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'src/server/worldserver') diff --git a/src/server/worldserver/RemoteAccess/RASocket.cpp b/src/server/worldserver/RemoteAccess/RASocket.cpp index 3e9c12b32d8..359abd39901 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.cpp +++ b/src/server/worldserver/RemoteAccess/RASocket.cpp @@ -33,6 +33,7 @@ RASocket::RASocket() { _minLevel = uint8(ConfigMgr::GetIntDefault("RA.MinLevel", 3)); + _commandExecuting = false; } RASocket::~RASocket() @@ -59,6 +60,13 @@ int RASocket::handle_close(ACE_HANDLE, ACE_Reactor_Mask) sLog->outInfo(LOG_FILTER_REMOTECOMMAND, "Closing connection"); peer().close_reader(); wait(); + // While the above wait() will wait for the ::svc() to finish, it will not wait for the async event + // RASocket::commandfinished to be completed. Calling destroy() before the latter function ends + // will lead to using a freed pointer -> crash. + while (_commandExecuting.value()) + ACE_OS::sleep(1); + + destroy(); return 0; } @@ -149,6 +157,7 @@ int RASocket::process_command(const std::string& command) return -1; } + _commandExecuting = true; CliCommandHolder* cmd = new CliCommandHolder(this, command.c_str(), &RASocket::zprint, &RASocket::commandFinished); sWorld->QueueCliCommand(cmd); @@ -411,12 +420,11 @@ void RASocket::commandFinished(void* callbackArg, bool /*success*/) // the message is 0 size control message to tell that command output is finished // hence we don't put timeout, because it shouldn't increase queue size and shouldn't block - if (socket->peer().get_handle() == ACE_INVALID_HANDLE // this can happen if this code is triggered when handle_close has already called peer().close_writer() - || socket->putq(mb->duplicate()) == -1) + if (socket->putq(mb->duplicate()) == -1) // getting here is bad, command can't be marked as complete sLog->outDebug(LOG_FILTER_REMOTECOMMAND, "Failed to enqueue command end message. Error is %s", ACE_OS::strerror(errno)); mb->release(); - socket->destroy(); // deletes the object - socket = NULL; + + socket->_commandExecuting = false; } diff --git a/src/server/worldserver/RemoteAccess/RASocket.h b/src/server/worldserver/RemoteAccess/RASocket.h index d23d1f0d5fd..e92cb35eaf0 100644 --- a/src/server/worldserver/RemoteAccess/RASocket.h +++ b/src/server/worldserver/RemoteAccess/RASocket.h @@ -57,6 +57,7 @@ class RASocket: public ACE_Svc_Handler private: /// Minimum security level required to connect uint8 _minLevel; + ACE_Atomic_Op _commandExecuting; }; #endif /// @} -- cgit v1.2.3