[Libreoffice-commits] online.git: loolwsd/ChildProcessSession.cpp loolwsd/ChildProcessSession.hpp loolwsd/LOOLKit.cpp loolwsd/LOOLSession.cpp loolwsd/LOOLSession.hpp loolwsd/LOOLWSD.cpp loolwsd/LOOLWSD.hpp loolwsd/MasterProcessSession.cpp loolwsd/MasterProcessSession.hpp loolwsd/Util.cpp loolwsd/Util.hpp

Ashod Nakashian ashod.nakashian at collabora.co.uk
Sun Dec 27 20:14:32 PST 2015


 loolwsd/ChildProcessSession.cpp  |    5 +++--
 loolwsd/ChildProcessSession.hpp  |    5 +++--
 loolwsd/LOOLKit.cpp              |    2 +-
 loolwsd/LOOLSession.cpp          |   18 +++++-------------
 loolwsd/LOOLSession.hpp          |   25 +++++++++++++++++++++----
 loolwsd/LOOLWSD.cpp              |   12 +++++++++---
 loolwsd/LOOLWSD.hpp              |   11 ++++++++++-
 loolwsd/MasterProcessSession.cpp |   32 +++++++++++++++++---------------
 loolwsd/MasterProcessSession.hpp |    6 ++++--
 loolwsd/Util.cpp                 |    7 +++++++
 loolwsd/Util.hpp                 |    3 +++
 11 files changed, 83 insertions(+), 43 deletions(-)

New commits:
commit 3811abb40a1004cf92aabdbbcac1557b64bdfa2e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Dec 27 22:47:39 2015 -0500

    loolwsd: replaced threadId with globally unique sessionId
    
    Change-Id: I55b16d4baa1753bc67dcd72de13e7a516da27abe
    Reviewed-on: https://gerrit.libreoffice.org/20981
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp
index 7ef8a39..7d22070 100644
--- a/loolwsd/ChildProcessSession.cpp
+++ b/loolwsd/ChildProcessSession.cpp
@@ -42,11 +42,12 @@ using Poco::URI;
 Poco::NotificationQueue ChildProcessSession::_callbackQueue;
 Poco::Mutex ChildProcessSession::_mutex;
 
-ChildProcessSession::ChildProcessSession(std::shared_ptr<WebSocket> ws,
+ChildProcessSession::ChildProcessSession(const std::string& id,
+                                         std::shared_ptr<Poco::Net::WebSocket> ws,
                                          LibreOfficeKit *loKit,
                                          LibreOfficeKitDocument * loKitDocument,
                                          const std::string& childId) :
-    LOOLSession(ws, Kind::ToMaster),
+    LOOLSession(id, Kind::ToMaster, ws),
     _loKitDocument(loKitDocument),
     _viewId(0),
     _loKit(loKit),
diff --git a/loolwsd/ChildProcessSession.hpp b/loolwsd/ChildProcessSession.hpp
index f40fd7e..2340f96 100644
--- a/loolwsd/ChildProcessSession.hpp
+++ b/loolwsd/ChildProcessSession.hpp
@@ -24,8 +24,9 @@ public:
     /// loKit The LOKit instance.
     /// loKitDocument The instance to an existing document (when opening
     ///                 a new view) or nullptr (when first view).
-    /// childId The id of the child, used by downloadas to construct jailed path.
-    ChildProcessSession(std::shared_ptr<Poco::Net::WebSocket> ws,
+    /// childId The id of the lokit instance, used by downloadas to construct jailed path.
+    ChildProcessSession(const std::string& id,
+                        std::shared_ptr<Poco::Net::WebSocket> ws,
                         LibreOfficeKit *loKit,
                         LibreOfficeKitDocument * loKitDocument,
                         const std::string& childId);
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index b61f0f9..5dd5b6a 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -384,7 +384,7 @@ public:
             HTTPResponse response;
             auto ws = std::make_shared<WebSocket>(cs, request, response);
 
-            _session.reset(new ChildProcessSession(ws, _loKit, _loKitDocument, _childId));
+            _session.reset(new ChildProcessSession(_threadId, ws, _loKit, _loKitDocument, _childId));
             ws->setReceiveTimeout(0);
 
             // child Jail TID PID
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 2e951c9..7538931 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -83,23 +83,15 @@ using Poco::Net::WebSocketException;
 
 const std::string LOOLSession::jailDocumentURL = "/user/thedocument";
 
-LOOLSession::LOOLSession(std::shared_ptr<WebSocket> ws, Kind kind) :
+LOOLSession::LOOLSession(const std::string& id, const Kind kind,
+                         std::shared_ptr<Poco::Net::WebSocket> ws) :
     _kind(kind),
+    _kindString(kind == Kind::ToClient ? "ToClient" :
+                kind == Kind::ToMaster ? "ToMaster" : "ToPrisoner"),
     _ws(ws),
     _docURL("")
 {
-    if (kind == Kind::ToClient)
-    {
-        _kindString = "ToClient";
-    }
-    else if (kind == Kind::ToMaster)
-    {
-        _kindString = "ToMaster";
-    }
-    else if (kind == Kind::ToPrisoner)
-    {
-        _kindString = "ToPrisoner";
-    }
+    setId(id);
 }
 
 LOOLSession::~LOOLSession()
diff --git a/loolwsd/LOOLSession.hpp b/loolwsd/LOOLSession.hpp
index b719ee8..85221ed 100644
--- a/loolwsd/LOOLSession.hpp
+++ b/loolwsd/LOOLSession.hpp
@@ -42,6 +42,9 @@ class LOOLSession
 public:
     enum class Kind { ToClient, ToPrisoner, ToMaster };
 
+    const std::string& getId() const { return _id; }
+    const std::string& getName() const { return _name; }
+
     void sendTextFrame(const std::string& text);
 
     virtual bool getStatus(const char *buffer, int length) = 0;
@@ -55,12 +58,15 @@ public:
     static const std::string jailDocumentURL;
 
 protected:
-    LOOLSession(std::shared_ptr<Poco::Net::WebSocket> ws, Kind kind);
+    LOOLSession(const std::string& id, const Kind kind,
+                std::shared_ptr<Poco::Net::WebSocket> ws);
     virtual ~LOOLSession();
 
-    const Kind _kind;
-
-    std::string _kindString;
+    void setId(const std::string& id)
+    {
+        _id = id;
+        _name = _kindString + '-' + id;
+    }
 
     void sendBinaryFrame(const char *buffer, int length);
 
@@ -75,6 +81,12 @@ protected:
 
     // Fields common to sessions in master and jailed processes:
 
+    // Our kind signifies to what we are connected to.
+    const Kind _kind;
+
+    // The kind cached as a string.
+    const std::string _kindString;
+
     // In the master process, the websocket to the LOOL client or the jailed child process. In a
     // jailed process, the websocket to the parent.
     std::shared_ptr<Poco::Net::WebSocket> _ws;
@@ -90,6 +102,11 @@ private:
     virtual bool _handleInput(const char *buffer, int length) = 0;
 
 private:
+    // A session ID specific to an end-to-end connection (from user to lokit).
+    std::string _id;
+    // A readable name that identifies our peer and ID.
+    std::string _name;
+
     std::mutex _mutex;
 };
 
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 683e304..fd8085f 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -344,8 +344,9 @@ public:
                 {
                     // Load the document.
                     std::shared_ptr<WebSocket> ws;
-                    LOOLSession::Kind kind = LOOLSession::Kind::ToClient;
-                    auto session = std::make_shared<MasterProcessSession>(ws, kind);
+                    const LOOLSession::Kind kind = LOOLSession::Kind::ToClient;
+                    const auto id = LOOLWSD::GenSessionId();
+                    auto session = std::make_shared<MasterProcessSession>(id, kind, ws);
                     const std::string filePrefix("file://");
                     const std::string load = "load url=" + filePrefix + fromPath;
                     session->handleInput(load.data(), load.size());
@@ -449,13 +450,17 @@ public:
                 auto ws = std::make_shared<WebSocket>(request, response);
 
                 LOOLSession::Kind kind;
+                std::string id;
 
                 if (request.getURI() == LOOLWSD::CHILD_URI && request.serverAddress().port() == MASTER_PORT_NUMBER)
                     kind = LOOLSession::Kind::ToPrisoner;
                 else
+                {
                     kind = LOOLSession::Kind::ToClient;
+                    id = LOOLWSD::GenSessionId();
+                }
 
-                auto session = std::make_shared<MasterProcessSession>(ws, kind);
+                auto session = std::make_shared<MasterProcessSession>(id, kind, ws);
 
                 // For ToClient sessions, we store incoming messages in a queue and have a separate
                 // thread that handles them. This is so that we can empty the queue when we get a
@@ -660,6 +665,7 @@ private:
     HTTPServer& _srv;
 };
 
+std::atomic<unsigned> LOOLWSD::NextSessionId;
 int LOOLWSD::timeoutCounter = 0;
 int LOOLWSD::writerBroker = -1;
 Poco::UInt64 LOOLWSD::_childId = 0;
diff --git a/loolwsd/LOOLWSD.hpp b/loolwsd/LOOLWSD.hpp
index 907fd85..7da3fa4 100644
--- a/loolwsd/LOOLWSD.hpp
+++ b/loolwsd/LOOLWSD.hpp
@@ -14,6 +14,7 @@
 
 #include <string>
 #include <mutex>
+#include <atomic>
 
 #include <Poco/Util/OptionSet.h>
 #include <Poco/Random.h>
@@ -22,6 +23,8 @@
 #include <Poco/SharedMemory.h>
 #include <Poco/NamedMutex.h>
 
+#include "Util.hpp"
+
 class LOOLWSD: public Poco::Util::ServerApplication
 {
 public:
@@ -30,6 +33,7 @@ public:
 
     // An Application is a singleton anyway, so just keep these as
     // statics
+    static std::atomic<unsigned> NextSessionId;
     static int timeoutCounter;
     static int _numPreSpawnedChildren;
     static int writerBroker;
@@ -49,6 +53,12 @@ public:
     static const std::string FIFO_FILE;
     static const std::string LOKIT_PIDLOG;
 
+    static
+    std::string GenSessionId()
+    {
+        return Util::encodeId(++NextSessionId, 4);
+    }
+
 protected:
     static void setSignals(bool bIgnore);
     static void handleSignal(int nSignal);
@@ -71,7 +81,6 @@ private:
     void startupBroker(int nBroker);
     int  createBroker();
 
-
 #if ENABLE_DEBUG
 public:
     static bool runningAsRoot;
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index aa7258a..c901449 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -44,12 +44,14 @@ using Poco::URI;
 
 std::map<Process::PID, UInt64> MasterProcessSession::_childProcesses;
 
-std::map<Thread::TID, std::shared_ptr<MasterProcessSession>> MasterProcessSession::_availableChildSessions;
+std::map<std::string, std::shared_ptr<MasterProcessSession>> MasterProcessSession::_availableChildSessions;
 std::mutex MasterProcessSession::_availableChildSessionMutex;
 std::condition_variable MasterProcessSession::_availableChildSessionCV;
 
-MasterProcessSession::MasterProcessSession(std::shared_ptr<WebSocket> ws, const Kind kind) :
-    LOOLSession(ws, kind),
+MasterProcessSession::MasterProcessSession(const std::string& id,
+                                           const Kind kind,
+                                           std::shared_ptr<Poco::Net::WebSocket> ws) :
+    LOOLSession(id, kind, ws),
     _childId(0),
     _pidChild(0),
     _curPart(0),
@@ -206,14 +208,14 @@ bool MasterProcessSession::_handleInput(const char *buffer, int length)
             return false;
         }
 
-        UInt64 childId = std::stoull(tokens[1]);
-        Thread::TID tId = std::stoull(tokens[2]);
-        Process::PID pidChild = std::stoull(tokens[3]);
+        const UInt64 childId = std::stoull(tokens[1]);
+        setId(tokens[2]);
+        const Process::PID pidChild = std::stoull(tokens[3]);
 
         std::unique_lock<std::mutex> lock(_availableChildSessionMutex);
-        _availableChildSessions.emplace(tId, shared_from_this());
+        _availableChildSessions.emplace(getId(), shared_from_this());
 
-        Log::info() << _kindString << " mapped " << this << " id=" << childId << ", tId=" << tId
+        Log::info() << _kindString << " mapped " << this << " childId=" << childId << ", id=" << getId()
                     << " into _availableChildSessions, size=" << _availableChildSessions.size() << Log::end;
 
         _childId = childId;
@@ -377,7 +379,7 @@ bool MasterProcessSession::loadDocument(const char* /*buffer*/, int /*length*/,
         URI aUri(_docURL);
 
         // request new URL session
-        const std::string aMessage = "request " + std::to_string(Thread::currentTid()) + " " + _docURL + "\r\n";
+        const std::string aMessage = "request " + getId() + " " + _docURL + "\r\n";
         Log::info("Sending to Broker: " + aMessage);
         Util::writeFIFO(LOOLWSD::writerBroker, aMessage.c_str(), aMessage.length());
     }
@@ -558,22 +560,22 @@ void MasterProcessSession::dispatchChild()
     std::shared_ptr<MasterProcessSession> childSession;
     std::unique_lock<std::mutex> lock(_availableChildSessionMutex);
 
-    Log::debug() << "Waiting for a child session permission for thread [" << Thread::currentTid() << "]." << Log::end;
+    Log::debug() << "Waiting for a child session permission for thread [" << getId() << "]." << Log::end;
     while (nRequest-- && !bFound)
     {
         _availableChildSessionCV.wait_for(
             lock,
             std::chrono::milliseconds(2000),
-            [&bFound]
+            [&bFound, this]
             {
-                return (bFound = _availableChildSessions.find(Thread::currentTid()) != _availableChildSessions.end());
+                return (bFound = _availableChildSessions.find(getId()) != _availableChildSessions.end());
             });
 
         if (!bFound)
         {
             Log::info() << "Retrying child permission... " << nRequest << Log::end;
             // request again new URL session
-            const std::string aMessage = "request " + std::to_string(Thread::currentTid()) + " " + _docURL + "\r\n";
+            const std::string aMessage = "request " + getId() + " " + _docURL + "\r\n";
             Util::writeFIFO(LOOLWSD::writerBroker, aMessage.c_str(), aMessage.length());
         }
     }
@@ -581,8 +583,8 @@ void MasterProcessSession::dispatchChild()
     if (bFound)
     {
         Log::debug("Waiting child session permission, done!");
-        childSession = _availableChildSessions[Thread::currentTid()];
-        _availableChildSessions.erase(Thread::currentTid());
+        childSession = _availableChildSessions[getId()];
+        _availableChildSessions.erase(getId());
     }
 
     lock.unlock();
diff --git a/loolwsd/MasterProcessSession.hpp b/loolwsd/MasterProcessSession.hpp
index 85cc047..0d6e3b7 100644
--- a/loolwsd/MasterProcessSession.hpp
+++ b/loolwsd/MasterProcessSession.hpp
@@ -19,7 +19,9 @@
 class MasterProcessSession final : public LOOLSession, public std::enable_shared_from_this<MasterProcessSession>
 {
 public:
-    MasterProcessSession(std::shared_ptr<Poco::Net::WebSocket> ws, Kind kind);
+    MasterProcessSession(const std::string& id,
+                         const Kind kind,
+                         std::shared_ptr<Poco::Net::WebSocket> ws);
     virtual ~MasterProcessSession();
 
     bool haveSeparateProcess();
@@ -63,7 +65,7 @@ public:
 
     // Sessions to pre-spawned child processes that have connected but are not yet assigned a
     // document to work on.
-    static std::map<Poco::Thread::TID, std::shared_ptr<MasterProcessSession>> _availableChildSessions;
+    static std::map<std::string, std::shared_ptr<MasterProcessSession>> _availableChildSessions;
     static std::mutex _availableChildSessionMutex;
     static std::condition_variable _availableChildSessionCV;
 
diff --git a/loolwsd/Util.cpp b/loolwsd/Util.cpp
index 1746767..17d07df 100644
--- a/loolwsd/Util.cpp
+++ b/loolwsd/Util.cpp
@@ -160,6 +160,13 @@ namespace Log
 
 namespace Util
 {
+    std::string encodeId(const unsigned number, const int padding)
+    {
+        std::ostringstream oss;
+        oss << std::hex << std::setw(padding) << std::setfill('0') << number;
+        return oss.str();
+    }
+
     bool windowingAvailable()
     {
 #ifdef __linux
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index d4a8640..6df29e3 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -28,6 +28,9 @@ namespace Util
        unsigned getNext();
     }
 
+    /// Encode an integral ID into a string, with padding support.
+    std::string encodeId(const unsigned number, const int padding = 5);
+
     bool windowingAvailable();
 
     // Sadly, older libpng headers don't use const for the pixmap pointer parameter to


More information about the Libreoffice-commits mailing list