[Libreoffice-commits] online.git: 2 commits - net/Socket.hpp

[Michael Meeks]

 net/Socket.hpp |   53 ++++++++++++++++++-----------------------------------
 1 file changed, 18 insertions(+), 35 deletions(-)

New commits:
commit a9d5801c37079a5acdec183559dc1a3b8525d2da
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Fri Mar 10 18:46:31 2017 +0000

    Unwind releaseSocket complexity.
    
    We only move sockets in response to input on the socket, which
    must happen in the thread that we're processing that input on.
    Ergo - no need for this complexity.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 488b6ec..0cd5c7c 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -313,17 +313,7 @@ public:
         Poco::Timestamp newNow;
         for (int i = static_cast<int>(size) - 1; i >= 0; --i)
         {
-            // First check if this is a removed socket.
-            // Polling from multiple threads is fine, but not invoking handlePoll.
-            auto it = std::find(_relSockets.begin(), _relSockets.end(), _pollSockets[i]);
-            if (it != _relSockets.end())
-            {
-                LOG_DBG("Releasing socket #" << _pollFds[i].fd << " (of " <<
-                        _pollSockets.size() << ") from " << _name);
-                _pollSockets.erase(_pollSockets.begin() + i);
-                _relSockets.erase(it);
-            }
-            else if (_pollFds[i].revents)
+            if (_pollFds[i].revents)
             {
                 Socket::HandleResult res = Socket::HandleResult::SOCKET_CLOSED;
                 try
@@ -425,16 +415,19 @@ public:
     void dumpState();
 
     /// Removes a socket from this poller.
+    /// NB. this must be called from the socket poll that
+    /// owns the socket.
     void releaseSocket(const std::shared_ptr<Socket>& socket)
     {
-//        assert(isCorrectThread());
-        if (socket)
-        {
-            std::lock_guard<std::mutex> lock(_mutex);
-            LOG_TRC("Queuing to release socket #" << socket->getFD() << " from " << _name);
-            _relSockets.emplace_back(socket);
-            wakeup();
-        }
+        assert(socket);
+        assert(isCorrectThread());
+        assert(socket->isCorrectThread(true));
+        auto it = std::find(_pollSockets.begin(), _pollSockets.end(), socket);
+        assert(it != _pollSockets.end());
+
+        LOG_TRC("Release socket #" << socket->getFD() << " from " << _name);
+
+        _pollSockets.erase(it);
     }
 
     const std::string& name() const { return _name; }
@@ -444,19 +437,6 @@ private:
     /// Initialize the poll fds array with the right events
     void setupPollFds(Poco::Timestamp &timeout)
     {
-        for (int i = static_cast<int>(_relSockets.size()) - 1; i >= 0; --i)
-        {
-            auto it = std::find(_pollSockets.begin(), _pollSockets.end(), _relSockets[i]);
-            if (it != _pollSockets.end())
-            {
-                LOG_DBG("Releasing socket #" << (*it)->getFD() << " (of " <<
-                        _pollSockets.size() << ") from " << _name);
-                _pollSockets.erase(it);
-            }
-        }
-
-        _relSockets.clear();
-
         const size_t size = _pollSockets.size();
 
         _pollFds.resize(size + 1); // + wakeup pipe
@@ -486,7 +466,6 @@ private:
     /// Protects _newSockets
     std::mutex _mutex;
     std::vector<std::shared_ptr<Socket>> _newSockets;
-    std::vector<std::shared_ptr<Socket>> _relSockets;
     std::vector<CallbackFn> _newCallbacks;
     /// The fds to poll.
     std::vector<pollfd> _pollFds;
commit 027b8110ed1bdfddf40d553a362787ca6ef8bfe0
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Fri Mar 10 18:46:16 2017 +0000

    Allow isCorrectThread asserts to be hard, or off by default.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 87b8dea..488b6ec 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -187,10 +187,14 @@ public:
 #endif
     }
 
-    virtual bool isCorrectThread()
+    virtual bool isCorrectThread(bool hard = false)
     {
 #if ENABLE_DEBUG
-        return !getenv("LOOL_CHECK_THREADS") || std::this_thread::get_id() == _owner;
+        bool sameThread = std::this_thread::get_id() == _owner;
+        if (hard)
+            return sameThread;
+        else
+            return !getenv("LOOL_CHECK_THREADS") || sameThread;
 #else
         return true;
 #endif