poll: simplify poll socket handling.

Restores exception guards reduces duplication simplifies code - to use existing disposition logic. fixes unusual branch on function whitespace removes logged warning on timeout removes timeout handling from 'checkRemoval' - timeout does not imply removal; return value needs re-evaluating there. restores checkTimeout in its original location.i - unclear what extra 'setClosed' is good for. also removes 'checkRemoval' from ServerSocket sub-class etc. Signed-off-by: Michael Meeks <[email protected]> Change-Id: Ibb582ec5b2600c7ac7cf3b6aecbee39c43b214f8
CollaboraOnline · Oct 26, 2024 · 749f035 · 749f035
1 parent 1594989
commit 749f035
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 26 deletions.
diff --git a/net/Socket.cpp b/net/Socket.cpp
@@ -678,25 +678,6 @@ int SocketPoll::poll(int64_t timeoutMaxMicroS)
                 // removed in a callback
                 ++itemsErased;
             }
-            else if (!_pollSockets[i]->isOpen())
-            {
-                // closed socket ..
-                ++itemsErased;
-                LOGA_TRC(Socket, '#' << _pollFds[i].fd << ": Removing socket (at " << i
-                         << " of " << _pollSockets.size() << ") from " << _name);
-                _pollSockets[i] = nullptr;
-            }
-            else if( _pollSockets[i]->checkRemoval(newNow) )
-            {
-                // timed out socket .. also checks
-                // ProtocolHandlerInterface
-                // - http::Session::checkTimeout() OK
-                // - WebSocketHandler::checkTimeout() OK
-                ++itemsErased;
-                LOGA_TRC(Socket, '#' << _pollFds[i].fd << ": Removing socket (at " << i
-                         << " of " << _pollSockets.size() << ") from " << _name);
-                _pollSockets[i] = nullptr;
-            }
             else if (_pollFds[i].fd == _pollSockets[i]->getFD())
             {
                 SocketDisposition disposition(_pollSockets[i]);
@@ -1534,13 +1515,6 @@ bool StreamSocket::checkRemoval(std::chrono::steady_clock::time_point now)
             return true;
         }
     }
-    if (_socketHandler && _socketHandler->checkTimeout(now))
-    {
-        assert(isOpen() == false); // should have issued shutdown
-        setClosed();
-        LOG_WRN("CheckRemoval: Timeout: " << getStatsString(now) << ", " << *this);
-        return true;
-    }
     return false;
 }
 

diff --git a/net/Socket.hpp b/net/Socket.hpp
@@ -1449,6 +1449,21 @@ class StreamSocket : public Socket,
     {
         ASSERT_CORRECT_SOCKET_THREAD(this);
 
+        if (_socketHandler->checkTimeout(now))
+        {
+            assert(isOpen() == false); // should have issued shutdown
+            setClosed();
+            LOGA_DBG(Socket, "socket timeout: " << getStatsString(now) << ", " << *this);
+            disposition.setClosed();
+            return;
+        }
+
+        if (!isOpen() || checkRemoval(now))
+        {
+            disposition.setClosed();
+            return;
+        }
+
         if (!events && _inBuffer.empty())
             return;