[Libreoffice-commits] online.git: 2 commits - net/Socket.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/LOOLWSD.cpp wsd/SenderQueue.hpp

[Michael Meeks]

 net/Socket.hpp        |    1 -
 wsd/ClientSession.cpp |    6 +-----
 wsd/ClientSession.hpp |    8 --------
 wsd/LOOLWSD.cpp       |    9 +++++++++
 wsd/SenderQueue.hpp   |   19 ++-----------------
 5 files changed, 12 insertions(+), 31 deletions(-)

New commits:
commit 737f7111b08ad4f395994769c45c69918ab0c337
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Wed Apr 5 21:01:48 2017 +0100

    Set thread owner to zero earlier to avoid race.
    
    Stop-gap fix, while we come up with a nice API for transferring
    sockets between SocketPolls, and/or detaching them generally.

diff --git a/net/Socket.hpp b/net/Socket.hpp
index 6bca6abf..6bfc6312 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -392,7 +392,6 @@ public:
             {
                 LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
                         _pollSockets.size() << ") from " << _name);
-                _pollSockets[i]->setThreadOwner(std::thread::id(0));
                 _pollSockets.erase(_pollSockets.begin() + i);
             }
         }
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 408d49d4..62879c1d 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1466,6 +1466,9 @@ private:
             _childProcess = child; // weak
             addNewChild(child);
 
+            // We no longer own this thread: FIXME.
+            socket->setThreadOwner(std::thread::id(0));
+
             // Remove from prisoner poll since there is no activity
             // until we attach the childProcess (with this socket)
             // to a docBroker, which will do the polling.
@@ -1862,6 +1865,9 @@ private:
                         // Make sure the thread is running before adding callback.
                         docBroker->startThread();
 
+                        // We no longer own this thread: FIXME.
+                        socket->setThreadOwner(std::thread::id(0));
+
                         docBroker->addCallback([docBroker, socket, clientSession, format]()
                         {
                             clientSession->setSaveAsSocket(socket);
@@ -2088,6 +2094,9 @@ private:
                 // Make sure the thread is running before adding callback.
                 docBroker->startThread();
 
+                // We no longer own this thread: FIXME.
+                socket->setThreadOwner(std::thread::id(0));
+
                 docBroker->addCallback([docBroker, socket, clientSession]()
                 {
                     // Set the ClientSession to handle Socket events.
commit 381bed9388d5cefcf3c82d349d29d0e744f73b83
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Wed Apr 5 18:06:58 2017 +0100

    Remove redundant structure, include, and _stop members.

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index d70d18c8..7ad8df31 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -37,8 +37,7 @@ ClientSession::ClientSession(const std::string& id,
     _docBroker(docBroker),
     _uriPublic(uriPublic),
     _isDocumentOwner(false),
-    _isAttached(false),
-    _stop(false)
+    _isAttached(false)
 {
     const size_t curConnections = ++LOOLWSD::NumConnections;
     LOG_INF("ClientSession ctor [" << getName() << "], current number of connections: " << curConnections);
@@ -48,8 +47,6 @@ ClientSession::~ClientSession()
 {
     const size_t curConnections = --LOOLWSD::NumConnections;
     LOG_INF("~ClientSession dtor [" << getName() << "], current number of connections: " << curConnections);
-
-    stop();
 }
 
 SocketHandlerInterface::SocketOwnership ClientSession::handleIncomingMessage()
@@ -790,7 +787,6 @@ void ClientSession::dumpState(std::ostream& os)
     os << "\t\tisReadOnly: " << isReadOnly()
        << "\n\t\tisDocumentOwner: " << _isDocumentOwner
        << "\n\t\tisAttached: " << _isAttached
-       << "\n\t\tstop: " <<_stop
        << "\n";
     _senderQueue.dumpState(os);
 }
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 3181b0c8..9cd67533 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -75,13 +75,6 @@ public:
         _senderQueue.enqueue(data);
     }
 
-    bool stopping() const { return _stop || _senderQueue.stopping(); }
-    void stop()
-    {
-        _stop = true;
-        _senderQueue.stop();
-    }
-
     /// Set the save-as socket which is used to send convert-to results.
     void setSaveAsSocket(const std::shared_ptr<StreamSocket>& socket)
     {
@@ -152,7 +145,6 @@ private:
     std::unique_ptr<WopiStorage::WOPIFileInfo> _wopiFileInfo;
 
     SenderQueue<std::shared_ptr<Message>> _senderQueue;
-    std::atomic<bool> _stop;
 };
 
 #endif
diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp
index af8199f1..41445ac9 100644
--- a/wsd/SenderQueue.hpp
+++ b/wsd/SenderQueue.hpp
@@ -26,31 +26,17 @@
 #include "Log.hpp"
 #include "TileDesc.hpp"
 
-#include "Socket.hpp" // FIXME: hack for wakeup-world ...
-
-struct SendItem
-{
-    std::shared_ptr<Message> Data;
-    std::string Meta;
-    std::chrono::steady_clock::time_point BirthTime;
-};
-
 /// A queue of data to send to certain Session's WS.
 template <typename Item>
 class SenderQueue final
 {
 public:
 
-    SenderQueue() :
-        _stop(false)
+    SenderQueue()
     {
     }
 
-    bool stopping() const { return _stop || TerminationFlag; }
-    void stop()
-    {
-        _stop = true;
-    }
+    bool stopping() const { return TerminationFlag; }
 
     size_t enqueue(const Item& item)
     {
@@ -173,7 +159,6 @@ private:
     mutable std::mutex _mutex;
     std::deque<Item> _queue;
     typedef typename std::deque<Item>::value_type queue_item_t;
-    std::atomic<bool> _stop;
 };
 
 #endif