More intelligent socket cleanup

src/game/WorldSocket.h

@@ -131,7 +131,7 @@ class WorldSocket : public MaNGOS::Socket
         void FinalizeSession() { m_sessionFinalized = true; }
 
         virtual bool Open() override;
-        virtual bool Deletable() const override { return m_sessionFinalized && Socket::Deletable(); }
+        virtual bool Deletable() const override { return (m_sessionFinalized || IsClosed()) && Socket::Deletable(); }
 
         /// Return the session key
         BigNumber &GetSessionKey() { return m_s; }

src/shared/Network/NetworkThread.hpp

@@ -36,7 +36,7 @@ namespace MaNGOS
     class NetworkThread
     {
         private:
-            const int WorkDelay = 500;
+            static const int WorkDelay = 500;
 
             boost::asio::io_service m_service;
 
@@ -47,6 +47,9 @@ namespace MaNGOS
             boost::asio::io_service::work m_work;
 
             std::mutex m_closingSocketLock;
+            // most people think that you should always use a vector rather than a list, but i believe that this is an exception
+            // because this collection can potentially get rather large and we will frequently be removing elements at an arbitrary
+            // position within it.
             std::list<std::unique_ptr<SocketType>> m_closingSockets;
 
             std::atomic<bool> m_pendingShutdown;
@@ -66,6 +69,16 @@ namespace MaNGOS
 
             ~NetworkThread()
             {
+                // we do not lock the list here because Close() will call RemoveSocket which needs the lock
+                while (!m_sockets.empty())
+                {
+                    // if it is already closed (which can happen with the placeholder socket for a pending accept), just remove it
+                    if (m_sockets.front()->IsClosed())
+                        m_sockets.erase(m_sockets.begin());
+                    else
+                        m_sockets.front()->Close();
+                }
+
                 m_pendingShutdown = true;
                 m_socketCleanupThread.join();
             }
@@ -83,6 +96,7 @@ namespace MaNGOS
                     if (i->get() == socket)
                     {
                         m_closingSockets.push_front(std::move(*i));
+                        m_sockets.erase(i);
                         return;
                     }
             }
@@ -91,14 +105,20 @@ namespace MaNGOS
     template <typename SocketType>
     void NetworkThread<SocketType>::SocketCleanupWork()
     {
-        while (!m_pendingShutdown)
+        while (!m_pendingShutdown || !m_closingSockets.empty())
         {
             {
                 std::lock_guard<std::mutex> guard(m_closingSocketLock);
 
                 for (auto i = m_closingSockets.begin(); i != m_closingSockets.end(); i++)
                     if (i->get()->Deletable())
+                    {
                         i = m_closingSockets.erase(i);
+
+                        // avoid incrementing the end iterator
+                        if (i == m_closingSockets.end())
+                            break;
+                    }
             }
 
             std::this_thread::sleep_for(std::chrono::milliseconds(WorkDelay));

src/shared/Network/Socket.cpp

@@ -83,6 +83,7 @@ void Socket::OnRead(const boost::system::error_code &error, size_t length)
 {
     if (error)
     {
+        m_readState = ReadState::Idle;
         OnError(error);
         return;
     }
@@ -152,7 +153,8 @@ void Socket::OnRead(const boost::system::error_code &error, size_t length)
 void Socket::OnError(const boost::system::error_code &error)
 {
     // skip logging this code because it happens whenever anyone disconnects.  reduces spam.
-    if (error != boost::asio::error::eof)
+    if (error != boost::asio::error::eof &&
+        error != boost::asio::error::operation_aborted)
         sLog.outBasic("Socket::OnError.  %s.  Connection closed.", error.message().c_str());
 
     if (!IsClosed())

src/shared/Network/Socket.hpp

@@ -35,7 +35,7 @@ namespace MaNGOS
         private:
             // buffer timeout period, in milliseconds.  higher values decrease responsiveness
             // ingame but increase bandwidth efficiency by reducing tcp overhead.
-            const int BufferTimeout = 50;
+            static const int BufferTimeout = 50;
 
             enum class WriteState
             {