[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/Util.hpp

2016-11-19 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |7 ++-
 loolwsd/DocumentBroker.hpp |7 ++-
 loolwsd/LOOLWSD.cpp|   11 ---
 loolwsd/Util.hpp   |   22 --
 4 files changed, 32 insertions(+), 15 deletions(-)

New commits:
commit d3e64b1aa478f6b6b5c62cfc5877c7bd29785645
Author: Ashod Nakashian 
Date:   Thu Nov 17 09:00:05 2016 -0500

loolwsd: improved alertAllUsers

More flexible reason message and other cleanups
to help use altertAllUsers in other situations.

Change-Id: I7f0c7b5ac21ffa55923f531d7b28f7537ef42dac
Reviewed-on: https://gerrit.libreoffice.org/30997
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 6cc4847..1b35377 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -517,7 +517,7 @@ size_t 
DocumentBroker::addSession(std::shared_ptr& session)
 // even if in this case it might be a totally different location (file 
system, or
 // some other type of storage somewhere). This message is not sent to 
all clients,
 // though, just to all sessions of this document.
-alertAllUsersOfDocument("internal", "diskfull");
+alertAllUsers("internal", "diskfull");
 throw;
 }
 
@@ -572,13 +572,10 @@ size_t DocumentBroker::removeSession(const std::string& 
id)
 return _sessions.size();
 }
 
-void DocumentBroker::alertAllUsersOfDocument(const std::string& cmd, const 
std::string& kind)
+void DocumentBroker::alertAllUsers(const std::string& msg)
 {
 Util::assertIsLocked(_mutex);
 
-std::stringstream ss;
-ss << "error: cmd=" << cmd << " kind=" << kind;
-const auto msg = ss.str();
 for (auto& it : _sessions)
 {
 it.second->sendTextFrame(msg);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 7e94fa2..6672aaf 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -242,7 +242,12 @@ public:
 /// Removes a session by ID. Returns the new number of sessions.
 size_t removeSession(const std::string& id);
 
-void alertAllUsersOfDocument(const std::string& cmd, const std::string& 
kind);
+void alertAllUsers(const std::string& msg);
+
+void alertAllUsers(const std::string& cmd, const std::string& kind)
+{
+alertAllUsers("error: cmd=" + cmd + " kind=" + kind);
+}
 
 /// Invalidate the cursor position.
 void invalidateCursor(int x, int y, int w, int h)
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 8a3fd40..0a34591 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -2078,7 +2078,7 @@ int LOOLWSD::main(const std::vector& 
/*args*/)
 // and wait until sockets close.
 LOG_INF("Stopping server socket listening. ShutdownFlag: " <<
 ShutdownFlag << ", TerminationFlag: " << TerminationFlag);
-Util::alertAllUsers("internal", "shutdown");
+Util::alertAllUsers("close: shutdown");
 
 srv.stop();
 srv2.stop();
@@ -2149,14 +2149,19 @@ namespace Util
 
 void alertAllUsers(const std::string& cmd, const std::string& kind)
 {
+alertAllUsers("error: cmd=" + cmd + " kind=" + kind);
+}
+
+void alertAllUsers(const std::string& msg)
+{
 std::lock_guard DocBrokersLock(DocBrokersMutex);
 
-LOG_INF("Alerting all users: cmd=" << cmd << ", kind=" << kind);
+LOG_INF("Alerting all users: [" << msg << "]");
 
 for (auto& brokerIt : DocBrokers)
 {
 auto lock = brokerIt.second->getLock();
-brokerIt.second->alertAllUsersOfDocument(cmd, kind);
+brokerIt.second->alertAllUsers(msg);
 }
 }
 
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index 40bcc30..e567972 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -47,14 +47,24 @@ namespace Util
 bool windowingAvailable();
 
 #ifndef BUILDING_TESTS
-// Send a 'error:' message with the specified cmd and kind parameters to 
all connected
-// clients. This function can be called either in loolwsd or loolkit 
processes, even if only
-// loolwsd obviously has contact with the actual clients; in loolkit it 
will be forwarded to
-// loolwsd for redistribution. (This function must be implemented 
separately in each program
-// that uses it, it is not in Util.cpp.)
+
+/// Send a message to all clients.
+void alertAllUsers(const std::string& msg);
+
+/// Send a 'error:' message with the specified cmd and kind parameters to 
all connected
+/// clients. This function can be called either in loolwsd or loolkit 
processes, even if only
+/// loolwsd obviously has contact with the actual clients; in loolkit it 
will be forwarded to
+/// loolwsd for redistribution. (This function must be implemented 
separately in each program
+/// that uses it, it is not in Util.cpp.)
 void alertAllUsers(const std::string& cmd, const std::string& kind);

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-11-06 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |9 +-
 loolwsd/DocumentBroker.hpp |6 +
 loolwsd/LOOLWSD.cpp|  147 ++---
 3 files changed, 81 insertions(+), 81 deletions(-)

New commits:
commit 3993757ee806d7bdde2408852f6227b8f848f1d8
Author: Ashod Nakashian 
Date:   Sat Nov 5 22:15:44 2016 -0400

loolwsd: fix race while creating new documents

This fixes the race rather than trying to patch it.
It still minimizes the locking necessary to a minimum
to maximize parallelism.

The approach is to have at least the DocBrokers lock
or the DocumentBroker lock (if we already have a doc)
while creating new document views.

Change-Id: I96b4f17b3be3d03cd5e6f4d17d39e2165fe008a7
Reviewed-on: https://gerrit.libreoffice.org/30628
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 97e21b4..19dbb97 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -358,9 +358,10 @@ bool DocumentBroker::save(const std::string& sessionId, 
bool success, const std:
 return false;
 }
 
-bool DocumentBroker::autoSave(const bool force, const size_t waitTimeoutMs)
+bool DocumentBroker::autoSave(const bool force, const size_t waitTimeoutMs, 
std::unique_lock& lock)
 {
-std::unique_lock lock(_mutex);
+Util::assertIsLocked(lock);
+
 if (_sessions.empty() || _storage == nullptr || !_isLoaded ||
 !_childProcess->isAlive() || (!_isModified && !force))
 {
@@ -549,7 +550,7 @@ bool 
DocumentBroker::connectPeers(std::shared_ptr& session)
 
 size_t DocumentBroker::removeSession(const std::string& id)
 {
-std::lock_guard lock(_mutex);
+Util::assertIsLocked(_mutex);
 
 auto it = _sessions.find(id);
 if (it != _sessions.end())
@@ -804,7 +805,7 @@ void DocumentBroker::handleTileCombinedResponse(const 
std::vector& payload
 
 bool DocumentBroker::startDestroy(const std::string& id)
 {
-std::unique_lock lock(_mutex);
+Util::assertIsLocked(_mutex);
 
 const auto currentSession = _sessions.find(id);
 assert(currentSession != _sessions.end());
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 17de968..e58b642 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -195,7 +195,7 @@ public:
 /// complete before returning, or timeout.
 /// @return true if attempts to save or it also waits
 /// and receives save notification. Otherwise, false.
-bool autoSave(const bool force, const size_t waitTimeoutMs);
+bool autoSave(const bool force, const size_t waitTimeoutMs, 
std::unique_lock& lock);
 
 Poco::URI getPublicUri() const { return _uriPublic; }
 Poco::URI getJailedUri() const { return _uriJailed; }
@@ -206,7 +206,7 @@ public:
 bool isAlive() const { return _childProcess && _childProcess->isAlive(); }
 size_t getSessionsCount() const
 {
-std::lock_guard lock(_mutex);
+Util::assertIsLocked(_mutex);
 return _sessions.size();
 }
 
@@ -273,6 +273,8 @@ public:
 /// Get the PID of the associated child process
 Poco::Process::PID getPid() const { return _childProcess->getPid(); }
 
+std::unique_lock getLock() { return 
std::unique_lock(_mutex); }
+
 private:
 /// Sends the .uno:Save command to LoKit.
 bool sendUnoSave(const bool dontSaveIfUnmodified);
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 9b91e1c..7ec35a1 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -271,6 +271,8 @@ static void forkChildren(const int number)
 /// Returns true if removed at least one.
 static bool cleanupChildren()
 {
+Util::assertIsLocked(NewChildrenMutex);
+
 bool removed = false;
 for (int i = NewChildren.size() - 1; i >= 0; --i)
 {
@@ -558,6 +560,7 @@ private:
 }
 
 docBrokersLock.lock();
+auto docLock = docBroker->getLock();
 sessionsCount = docBroker->removeSession(id);
 if (sessionsCount == 0)
 {
@@ -723,93 +726,92 @@ private:
 cleanupDocBrokers();
 
 // Lookup this document.
-auto it = DocBrokers.find(docKey);
-if (it != DocBrokers.end())
+auto it = DocBrokers.lower_bound(docKey);
+if (it != DocBrokers.end() && it->first == docKey)
 {
 // Get the DocumentBroker from the Cache.
 Log::debug("Found DocumentBroker for docKey [" + docKey + "].");
 docBroker = it->second;
 assert(docBroker);
-}
-else
-{
-// New Document.
-#if MAX_DOCUMENTS > 0
-if (DocBrokers.size() + 1 > MAX_DOCUMENTS)
-{
-Log::error() << "Limit on maximum number of open documents of "
- << MAX_DOCUMENTS

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-11-06 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   23 +++
 loolwsd/DocumentBroker.hpp |7 +++
 loolwsd/LOOLWSD.cpp|   25 ++---
 3 files changed, 48 insertions(+), 7 deletions(-)

New commits:
commit 800e711321e19b02f5e15f496525d42edcba4376
Author: Ashod Nakashian 
Date:   Sat Nov 5 23:00:16 2016 -0400

loolwsd: proper ChildProcess cleanup

Change-Id: If9be827aa50471b7d1d922402414d028ccdcff8b
Reviewed-on: https://gerrit.libreoffice.org/30629
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 19dbb97..507362c 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -180,6 +180,10 @@ DocumentBroker::~DocumentBroker()
 {
 LOG_WRN("DocumentBroker still has unremoved sessions.");
 }
+
+// Need to first make sure the child exited, socket closed,
+// and thread finished before we are destroyed.
+_childProcess.reset();
 }
 
 bool DocumentBroker::load(const std::string& sessionId, const std::string& 
jailId)
@@ -922,4 +926,23 @@ void DocumentBroker::childSocketTerminated()
 }
 }
 
+void DocumentBroker::terminateChild(std::unique_lock& lock)
+{
+Util::assertIsLocked(_mutex);
+Util::assertIsLocked(lock);
+
+Log::info() << "Terminating child [" << getPid() << "] of doc [" << 
_docKey << "]." << Log::end;
+
+assert(_sessions.empty() && "DocumentBroker still has unremoved 
sessions!");
+
+// First flag to stop as it might be waiting on our lock
+// to process some incoming message.
+_childProcess->stop();
+
+// Release the lock and wait for the thread to finish.
+lock.unlock();
+
+_childProcess->close(false);
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index e58b642..0075fb6 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -270,6 +270,13 @@ public:
 /// or upon failing to process an incoming message.
 void childSocketTerminated();
 
+/// This gracefully terminates the connection
+/// with the child and cleans up ChildProcess etc.
+/// We must be called under lock and it must be
+/// passed to us so we unlock before waiting on
+/// the ChildProcess thread, which can take our lock.
+void terminateChild(std::unique_lock& lock);
+
 /// Get the PID of the associated child process
 Poco::Process::PID getPid() const { return _childProcess->getPid(); }
 
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 7ec35a1..ed5810a 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -565,11 +565,9 @@ private:
 if (sessionsCount == 0)
 {
 // At this point we're done.
-// We can't save if we hadn't, just kill.
-Log::debug("Closing child for docKey [" + docKey + 
"].");
-child->close(true);
-Log::debug("Removing DocumentBroker for docKey [" + 
docKey + "].");
+LOG_DBG("Removing DocumentBroker for docKey [" << 
docKey << "].");
 DocBrokers.erase(docKey);
+docBroker->terminateChild(docLock);
 }
 else
 {
@@ -928,9 +926,22 @@ private:
 
 if (sessionsCount == 0)
 {
-std::unique_lock DocBrokersLock(DocBrokersMutex);
-Log::debug("Removing DocumentBroker for docKey [" + docKey + 
"].");
-DocBrokers.erase(docKey);
+// We've supposedly destroyed the last session and can do away 
with
+// DocBroker. But first we need to take both locks in the 
correct
+// order and check again. We can't take the DocBrokersMutex 
while
+// holding the docBroker lock as that can deadlock with 
autoSave below.
+std::unique_lock docBrokersLock2(DocBrokersMutex);
+it = DocBrokers.find(docKey);
+if (it != DocBrokers.end() && it->second)
+{
+auto lock = it->second->getLock();
+if (it->second->getSessionsCount() == 0)
+{
+Log::info("Removing DocumentBroker for docKey [" + 
docKey + "].");
+DocBrokers.erase(docKey);
+docBroker->terminateChild(lock);
+}
+}
 }
 
 LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "EndSession: " 
+ uri);
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-10-16 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |4 ++--
 loolwsd/DocumentBroker.hpp |2 +-
 loolwsd/LOOLWSD.cpp|   10 --
 3 files changed, 7 insertions(+), 9 deletions(-)

New commits:
commit 7043b95f4d9a1d01043f6b519d5845a52375f1d2
Author: Ashod Nakashian 
Date:   Fri Oct 14 22:52:33 2016 -0400

loolwsd: fix convert-to after removing Prisoner WS

Change-Id: I652a9fffa267e01043262eb25d3c2e19bf9eb447
Reviewed-on: https://gerrit.libreoffice.org/29940
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index fd29bf5..77aa6ba 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -126,12 +126,12 @@ DocumentBroker::DocumentBroker() :
 DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
const std::string& docKey,
const std::string& childRoot,
-   std::shared_ptr childProcess) :
+   const std::shared_ptr& 
childProcess) :
 _uriPublic(uriPublic),
 _docKey(docKey),
 _childRoot(childRoot),
 _cacheRoot(getCachePath(uriPublic.toString())),
-_childProcess(std::move(childProcess)),
+_childProcess(childProcess),
 _lastSaveTime(std::chrono::steady_clock::now()),
 _markToDestroy(false),
 _lastEditableSession(false),
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 00d27e2..d5e8b42 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -171,7 +171,7 @@ public:
 DocumentBroker(const Poco::URI& uriPublic,
const std::string& docKey,
const std::string& childRoot,
-   std::shared_ptr childProcess);
+   const std::shared_ptr& childProcess);
 
 ~DocumentBroker()
 {
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 957749f..f709749 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -472,17 +472,11 @@ private:
 std::shared_ptr ws;
 auto session = std::make_shared(id, ws, 
docBroker, uriPublic);
 
-// Request the child to connect to us and add this session.
 auto sessionsCount = docBroker->addSession(session);
 Log::trace(docKey + ", ws_sessions++: " + 
std::to_string(sessionsCount));
 
 lock.unlock();
 
-// Wait until the client has connected with a prison 
socket.
-waitBridgeCompleted(session);
-// Now the bridge between the client and kit processes is 
connected
-// Let messages flow
-
 std::string encodedFrom;
 URI::encode(docBroker->getPublicUri().getPath(), "", 
encodedFrom);
 const std::string load = "load url=" + encodedFrom;
@@ -524,6 +518,10 @@ private:
 sessionsCount = docBroker->removeSession(id);
 if (sessionsCount == 0)
 {
+// At this point we're done.
+// We can't save if we hadn't, just kill.
+Log::debug("Closing child for docKey [" + docKey + 
"].");
+child->close(true);
 Log::debug("Removing DocumentBroker for docKey [" + 
docKey + "].");
 docBrokers.erase(docKey);
 }
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-10-10 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   39 ---
 loolwsd/DocumentBroker.hpp |   16 +++-
 loolwsd/LOOLWSD.cpp|9 +
 3 files changed, 32 insertions(+), 32 deletions(-)

New commits:
commit 22fb71c6729cb9139714054bf7096619c3c0aa1e
Author: Ashod Nakashian 
Date:   Sat Oct 8 10:31:35 2016 -0400

loolwsd: cleanup lastEditableSession flag in DocumentBroker

Change-Id: Iab3a226e3ff089c6bdab264d6f1b1d639bcf3b37
Reviewed-on: https://gerrit.libreoffice.org/29630
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index d6aba21..896e7e4 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -109,12 +109,12 @@ DocumentBroker::DocumentBroker() :
 _lastSaveTime(std::chrono::steady_clock::now()),
 _markToDestroy(true),
 _lastEditableSession(true),
+_isLoaded(false),
+_isModified(false),
 _cursorPosX(0),
 _cursorPosY(0),
 _cursorWidth(0),
 _cursorHeight(0),
-_isLoaded(false),
-_isModified(false),
 _mutex(),
 _saveMutex(),
 _tileVersion(0)
@@ -134,12 +134,12 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _lastSaveTime(std::chrono::steady_clock::now()),
 _markToDestroy(false),
 _lastEditableSession(false),
+_isLoaded(false),
+_isModified(false),
 _cursorPosX(0),
 _cursorPosY(0),
 _cursorWidth(0),
 _cursorHeight(0),
-_isLoaded(false),
-_isModified(false),
 _mutex(),
 _saveMutex(),
 _tileVersion(0)
@@ -244,7 +244,7 @@ bool DocumentBroker::save(bool success, const std::string& 
result)
 // If we aren't destroying the last editable session just yet, and the file
 // timestamp hasn't changed, skip saving.
 const auto newFileModifiedTime = 
Poco::File(_storage->getLocalRootPath()).getLastModified();
-if (!isLastEditableSession() && newFileModifiedTime == 
_lastFileModifiedTime)
+if (!_lastEditableSession && newFileModifiedTime == _lastFileModifiedTime)
 {
 // Nothing to do.
 Log::debug() << "Skipping unnecessary saving to URI [" << uri
@@ -409,7 +409,7 @@ size_t 
DocumentBroker::addSession(std::shared_ptr& session)
 }
 
 // Below values are recalculated when startDestroy() is called (before 
destroying the
-// document). It is safe to reset their values to their defaults whenever 
a new session is added
+// document). It is safe to reset their values to their defaults whenever 
a new session is added.
 _lastEditableSession = false;
 _markToDestroy = false;
 
@@ -683,31 +683,32 @@ void DocumentBroker::handleTileCombinedResponse(const 
std::vector& payload
 }
 }
 
-void DocumentBroker::startDestroy(const std::string& id)
+bool DocumentBroker::startDestroy(const std::string& id)
 {
 std::unique_lock lock(_mutex);
 
-auto currentSession = _sessions.find(id);
+const auto currentSession = _sessions.find(id);
 assert(currentSession != _sessions.end());
 
-// Check if session which is being destroyed is last non-readonly session
-bool lastEditableSession = !currentSession->second->isReadOnly();
-for (auto& it: _sessions)
+// Check if the session being destroyed is the last non-readonly session 
or not.
+_lastEditableSession = !currentSession->second->isReadOnly();
+if (_lastEditableSession && !_sessions.empty())
 {
-if (it.second->getId() == id)
-continue;
-
-if (!it.second->isReadOnly())
+for (const auto& it: _sessions)
 {
-lastEditableSession = false;
+if (it.second->getId() != id &&
+!it.second->isReadOnly())
+{
+// Found another editable.
+_lastEditableSession = false;
+break;
+}
 }
 }
 
-// Last editable session going away
-_lastEditableSession = lastEditableSession;
-
 // Last view going away, can destroy.
 _markToDestroy = (_sessions.size() <= 1);
+return _lastEditableSession;
 }
 
 void DocumentBroker::setModified(const bool value)
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 16008f4..4909e34 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -217,18 +217,16 @@ public:
const std::shared_ptr& session);
 void handleTileCombinedRequest(TileCombined& tileCombined,
const std::shared_ptr& 
session);
-
 void cancelTileRequests(const std::shared_ptr& session);
-
 void handleTileResponse(const std::vector& payload);
 void handleTileCombinedResponse(const std::vector& payload);
 
-// Called before destroying any session
-// This method calculates and sets important states of
-// session being destroyed.
-void

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-09-15 Thread Jan Holesovsky 
 loolwsd/DocumentBroker.cpp |   21 +
 loolwsd/DocumentBroker.hpp |3 +++
 loolwsd/LOOLWSD.cpp|   11 ++-
 3 files changed, 34 insertions(+), 1 deletion(-)

New commits:
commit 27bed804d5c33ed5c49fc648ccc9b1bb2a3c54a7
Author: Jan Holesovsky 
Date:   Thu Sep 15 12:41:57 2016 +0200

bccu#2005: Make sure we don't create 2 jails for the same document.

When 2 users opened the document at the very same time, it happened that new
jails / instances were created for the document twice.

As a solution, insert a dummy (marked to destroy) document into the map of
DocumentBrokers, which will lead into synchronization between the two
instances.

[I suppose the synchronization did not work previously either, as emplace()
does not seem to modify the value when the key is already in the map.]

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 043218f..ff3fc1b 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -97,6 +97,27 @@ std::string DocumentBroker::getDocKey(const Poco::URI& uri)
 return docKey;
 }
 
+DocumentBroker::DocumentBroker() :
+_uriPublic(),
+_docKey(),
+_childRoot(),
+_cacheRoot(),
+_childProcess(),
+_lastSaveTime(std::chrono::steady_clock::now()),
+_markToDestroy(true),
+_lastEditableSession(true),
+_cursorPosX(0),
+_cursorPosY(0),
+_cursorWidth(0),
+_cursorHeight(0),
+_isLoaded(false),
+_isModified(false),
+_isEditLockHeld(false),
+_tileVersion(0)
+{
+Log::info("Empty DocumentBroker (marked to destroy) created.");
+}
+
 DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
const std::string& docKey,
const std::string& childRoot,
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index fd28ed8..301f896 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -136,6 +136,9 @@ public:
 static
 std::string getDocKey(const Poco::URI& uri);
 
+/// Dummy document broker that is marked to destroy.
+DocumentBroker();
+
 DocumentBroker(const Poco::URI& uriPublic,
const std::string& docKey,
const std::string& childRoot,
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index c2c7068..91746f8 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -565,6 +565,15 @@ private:
 docBroker = it->second;
 assert(docBroker);
 }
+else
+{
+// Store a dummy (marked to destroy) document broker until we
+// have the real one, so that the other requests block
+Log::debug("Inserting a dummy DocumentBroker for docKey [" + 
docKey + "] temporarily.");
+
+std::shared_ptr tempBroker = 
std::make_shared();
+docBrokers.emplace(docKey, tempBroker);
+}
 }
 
 if (docBroker)
@@ -645,7 +654,7 @@ private:
 if (newDoc)
 {
 std::unique_lock lock(docBrokersMutex);
-docBrokers.emplace(docKey, docBroker);
+docBrokers[docKey] = docBroker;
 }
 
 // Check if readonly session is required
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/Storage.cpp loolwsd/Storage.hpp

2016-08-29 Thread Pranav Kant 
 loolwsd/DocumentBroker.cpp |8 ++--
 loolwsd/DocumentBroker.hpp |2 +-
 loolwsd/LOOLWSD.cpp|4 ++--
 loolwsd/Storage.cpp|   10 +++---
 loolwsd/Storage.hpp|2 ++
 5 files changed, 18 insertions(+), 8 deletions(-)

New commits:
commit f8ebb54af0948dbd4b9cb7bdceb44d90b190f4f9
Author: Pranav Kant 
Date:   Mon Aug 29 19:11:37 2016 +0530

loolwsd: Receive WOPI userid, username

Change-Id: I0bd5e5a155b8f8486fbeffb1c1413d5e9c177fc3

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index e3f1954..d119f9c 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -122,16 +122,20 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: 
[" + _docKey + "]");
 }
 
-void DocumentBroker::validate(const Poco::URI& uri)
+const StorageBase::FileInfo DocumentBroker::validate(const Poco::URI& uri)
 {
 Log::info("Validating: " + uri.toString());
 try
 {
 auto storage = StorageBase::create("", "", uri);
-if (storage == nullptr || !storage->getFileInfo(uri).isValid())
+auto fileinfo = storage->getFileInfo(uri);
+Log::info("After checkfileinfo: " + fileinfo._filename);
+if (storage == nullptr || !fileinfo.isValid())
 {
 throw BadRequestException("Invalid URI or access denied.");
 }
+
+return fileinfo;
 }
 catch (const std::exception&)
 {
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 1324005..e404e82 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -148,7 +148,7 @@ public:
 << " sessions left." << Log::end;
 }
 
-void validate(const Poco::URI& uri);
+const StorageBase::FileInfo validate(const Poco::URI& uri);
 
 /// Loads a document from the public URI into the jail.
 bool load(const std::string& jailId);
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 9a38e9a..21e867b 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -641,8 +641,8 @@ private:
 }
 
 // Validate the URI and Storage before moving on.
-docBroker->validate(uriPublic);
-Log::debug("Validated [" + uriPublic.toString() + "].");
+const auto fileinfo = docBroker->validate(uriPublic);
+Log::debug("Validated [" + uriPublic.toString() + "] requested with 
userid [" + fileinfo._userId + "] and username [" + fileinfo._userName + "]");
 
 if (newDoc)
 {
diff --git a/loolwsd/Storage.cpp b/loolwsd/Storage.cpp
index f44482e..7deba6a 100644
--- a/loolwsd/Storage.cpp
+++ b/loolwsd/Storage.cpp
@@ -175,7 +175,7 @@ StorageBase::FileInfo LocalStorage::getFileInfo(const 
Poco::URI& uri)
 const auto file = Poco::File(path);
 const auto lastModified = file.getLastModified();
 const auto size = file.getSize();
-return FileInfo({filename, lastModified, size});
+return FileInfo({filename, lastModified, size, "localhost", "Local Host"});
 }
 
 std::string LocalStorage::loadStorageFileToLocal()
@@ -276,6 +276,8 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const 
Poco::URI& uri)
 // Parse the response.
 std::string filename;
 size_t size = 0;
+std::string userId;
+std::string userName;
 std::string resMsg;
 Poco::StreamCopier::copyToString(rs, resMsg);
 Log::debug("WOPI::CheckFileInfo returned: " + resMsg);
@@ -288,10 +290,12 @@ StorageBase::FileInfo WopiStorage::getFileInfo(const 
Poco::URI& uri)
 const auto& object = result.extract();
 filename = object->get("BaseFileName").toString();
 size = std::stoul (object->get("Size").toString(), nullptr, 0);
+userId = object->get("UserId").toString();
+userName = object->get("UserFriendlyName").toString();
 }
 
 // WOPI doesn't support file last modified time.
-return FileInfo({filename, Poco::Timestamp(), size});
+return FileInfo({filename, Poco::Timestamp(), size, userId, userName});
 }
 
 /// uri format: http://server/<...>/wopi*/files//content
@@ -387,7 +391,7 @@ StorageBase::FileInfo WebDAVStorage::getFileInfo(const 
Poco::URI& uri)
 Log::debug("Getting info for webdav uri [" + uri.toString() + "].");
 (void)uri;
 assert(false && "Not Implemented!");
-return FileInfo({"bazinga", Poco::Timestamp(), 0});
+return FileInfo({"bazinga", Poco::Timestamp(), 0, "admin", "admin"});
 }
 
 std::string WebDAVStorage::loadStorageFileToLocal()
diff --git a/loolwsd/Storage.hpp b/loolwsd/Storage.hpp
index d4db4e4..4f7e120 100644
--- a/loolwsd/Storage.hpp
+++ b/loolwsd/Storage.hpp
@@ -39,6 +39,8 @@ public:
 std::string _filename;
 Poco::Timestamp _modifiedTime;
 size_t _size;
+std::string _userId;
+std::string _userName;
 };
 
 /// localStorePath the absolute root path of the chroot.

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-05-16 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |3 ++-
 loolwsd/DocumentBroker.hpp |5 +++--
 loolwsd/LOOLWSD.cpp|6 +++---
 3 files changed, 8 insertions(+), 6 deletions(-)

New commits:
commit 7c76e4b6bcdd7da3e6985a77445c1db668c11734
Author: Ashod Nakashian 
Date:   Mon May 16 07:46:27 2016 -0400

loolwsd: MasterProcessSession splitting: using ClientSession

Change-Id: I2ee089c04d2e5fbdae91cfc5cada437f5aae8e5b
Reviewed-on: https://gerrit.libreoffice.org/25038
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index e3735f9..19eaa5b 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -18,6 +18,7 @@
 #include "Storage.hpp"
 #include "TileCache.hpp"
 #include "LOOLProtocol.hpp"
+#include "ClientSession.hpp"
 #include "PrisonerSession.hpp"
 
 using namespace LOOLProtocol;
@@ -314,7 +315,7 @@ void DocumentBroker::takeEditLock(const std::string& id)
 }
 }
 
-size_t DocumentBroker::addSession(std::shared_ptr& 
session)
+size_t DocumentBroker::addSession(std::shared_ptr& session)
 {
 const auto id = session->getId();
 const std::string aMessage = "session " + id + " " + _docKey + "\n";
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 20b9761..a41778d 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -117,6 +117,7 @@ private:
 };
 
 class PrisonerSession;
+class ClientSession;
 
 /// DocumentBroker is responsible for setting up a document
 /// in jail and brokering loading it from Storage
@@ -193,7 +194,7 @@ public:
 void takeEditLock(const std::string& id);
 
 /// Add a new session. Returns the new number of sessions.
-size_t addSession(std::shared_ptr& session);
+size_t addSession(std::shared_ptr& session);
 /// Connect a prison session to its client peer.
 bool connectPeers(std::shared_ptr& session);
 /// Removes a session by ID. Returns the new number of sessions.
@@ -229,7 +230,7 @@ private:
 std::string _filename;
 std::chrono::steady_clock::time_point _lastSaveTime;
 Poco::Timestamp _lastFileModifiedTime;
-std::map _sessions;
+std::map _sessions;
 std::unique_ptr _storage;
 std::unique_ptr _tileCache;
 std::atomic _markToDestroy;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 358c152..f6a6fe6 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -384,7 +384,7 @@ private:
 
 // Load the document.
 std::shared_ptr ws;
-auto session = std::make_shared(id, 
LOOLSession::Kind::ToClient, ws, docBroker, nullptr);
+auto session = std::make_shared(id, 
LOOLSession::Kind::ToClient, ws, docBroker, nullptr);
 
 // Request the child to connect to us and add this session.
 auto sessionsCount = docBroker->addSession(session);
@@ -610,13 +610,13 @@ private:
 
 // Above this point exceptions are safe and will auto-cleanup.
 // Below this, we need to cleanup internal references.
-std::shared_ptr session;
+std::shared_ptr session;
 try
 {
 // For ToClient sessions, we store incoming messages in a queue 
and have a separate
 // thread to pump them. This is to empty the queue when we get a 
"canceltiles" message.
 auto queue = std::make_shared();
-session = std::make_shared(id, 
LOOLSession::Kind::ToClient, ws, docBroker, queue);
+session = std::make_shared(id, 
LOOLSession::Kind::ToClient, ws, docBroker, queue);
 
 // Request the child to connect to us and add this session.
 auto sessionsCount = docBroker->addSession(session);
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-05-04 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |1 +
 loolwsd/DocumentBroker.hpp |   21 -
 loolwsd/LOOLWSD.cpp|1 +
 3 files changed, 22 insertions(+), 1 deletion(-)

New commits:
commit ff0d4a3a9119de9bd65ef03c7ae32ba54d532b14
Author: Ashod Nakashian 
Date:   Mon May 2 07:21:30 2016 -0400

Revert "Revert "loolwsd: establish communication with...

...child from DocumentBroker""

Restore the communication with child from DocumentBroker.

This reverts commit 20ab6e8ae70254557e5bff242dbb9d5861fa946c.

Change-Id: I248bededff7074d8fb482b2cdd172048f80c02b2
Reviewed-on: https://gerrit.libreoffice.org/24639
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 5dbdf46..0cc1975 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -79,6 +79,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
+
 Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: 
[" + _docKey + "]");
 }
 
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 965ced3..49b5d32 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -39,8 +40,10 @@ public:
 /// @param ws is the control WebSocket to the child.
 ChildProcess(const Poco::Process::PID pid, const 
std::shared_ptr& ws) :
 _pid(pid),
-_ws(ws)
+_ws(ws),
+_stop(false)
 {
+_thread = std::thread([this]() { this->socketProcessor(); });
 Log::info("ChildProcess ctor [" + std::to_string(_pid) + "].");
 }
 
@@ -57,8 +60,16 @@ public:
 }
 }
 
+void setDocumentBroker(const std::shared_ptr& docBroker)
+{
+_docBroker = docBroker;
+}
+
 void close(const bool rude)
 {
+_stop = true;
+IoUtil::shutdownWebSocket(_ws);
+_thread.join();
 _ws.reset();
 if (_pid != -1)
 {
@@ -95,8 +106,14 @@ public:
 }
 
 private:
+void socketProcessor();
+
+private:
 Poco::Process::PID _pid;
 std::shared_ptr _ws;
+std::weak_ptr _docBroker;
+std::thread _thread;
+std::atomic _stop;
 };
 
 /// DocumentBroker is responsible for setting up a document
@@ -184,6 +201,8 @@ public:
 bool canDestroy();
 bool isMarkedToDestroy() const { return _markToDestroy; }
 
+bool handleInput(const std::vector& payload);
+
 private:
 
 /// Sends the .uno:Save command to LoKit.
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index d4d8dcc..5af6964 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -558,6 +558,7 @@ private:
 // Set one we just created.
 Log::debug("New DocumentBroker for docKey [" + docKey + "].");
 docBroker = std::make_shared(uriPublic, docKey, 
LOOLWSD::ChildRoot, child);
+child->setDocumentBroker(docBroker);
 }
 
 // Validate the broker.
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-05-02 Thread Jan Holesovsky 
 loolwsd/DocumentBroker.cpp |   30 --
 loolwsd/DocumentBroker.hpp |   20 +---
 loolwsd/LOOLWSD.cpp|1 -
 3 files changed, 1 insertion(+), 50 deletions(-)

New commits:
commit 20ab6e8ae70254557e5bff242dbb9d5861fa946c
Author: Jan Holesovsky 
Date:   Mon May 2 11:49:03 2016 +0200

Revert "loolwsd: establish communication with child from DocumentBroker"

Unfortunately this causes a deadlock in the unit tests.

This reverts commit 10417c9447ec1d34a8a599daf28ac4339a37930a.

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 52e30f0..8f50e32 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -17,27 +17,6 @@
 #include "LOOLWSD.hpp"
 #include "Storage.hpp"
 #include "TileCache.hpp"
-#include "LOOLProtocol.hpp"
-
-using namespace LOOLProtocol;
-
-void ChildProcess::socketProcessor()
-{
-IoUtil::SocketProcessor(_ws,
-[this](const std::vector& payload)
-{
-auto docBroker = this->_docBroker.lock();
-if (docBroker)
-{
-return docBroker->handleInput(payload);
-}
-
-Log::warn("No DocumentBroker to handle child message: [" + 
LOOLProtocol::getAbbreviatedMessage(payload) + "].");
-return true;
-},
-[]() { },
-[this]() { return !!this->_stop; });
-}
 
 namespace
 {
@@ -100,7 +79,6 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
-
 Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: 
[" + _docKey + "]");
 }
 
@@ -381,14 +359,6 @@ size_t DocumentBroker::removeSession(const std::string& id)
 return _sessions.size();
 }
 
-bool DocumentBroker::handleInput(const std::vector& payload)
-{
-Log::trace("DocumentBroker got child message: [" + 
LOOLProtocol::getAbbreviatedMessage(payload) + "].");
-
-//TODO: Handle message.
-return true;
-}
-
 bool DocumentBroker::canDestroy()
 {
 std::unique_lock lock(_mutex);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index efdedc3..965ced3 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -17,7 +17,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include 
@@ -40,10 +39,8 @@ public:
 /// @param ws is the control WebSocket to the child.
 ChildProcess(const Poco::Process::PID pid, const 
std::shared_ptr& ws) :
 _pid(pid),
-_ws(ws),
-_stop(false)
+_ws(ws)
 {
-_thread = std::thread([this]() { this->socketProcessor(); });
 Log::info("ChildProcess ctor [" + std::to_string(_pid) + "].");
 }
 
@@ -60,15 +57,8 @@ public:
 }
 }
 
-void setDocumentBroker(const std::shared_ptr& docBroker)
-{
-_docBroker = docBroker;
-}
-
 void close(const bool rude)
 {
-_stop = true;
-_thread.join();
 _ws.reset();
 if (_pid != -1)
 {
@@ -105,14 +95,8 @@ public:
 }
 
 private:
-void socketProcessor();
-
-private:
 Poco::Process::PID _pid;
 std::shared_ptr _ws;
-std::weak_ptr _docBroker;
-std::thread _thread;
-std::atomic _stop;
 };
 
 /// DocumentBroker is responsible for setting up a document
@@ -200,8 +184,6 @@ public:
 bool canDestroy();
 bool isMarkedToDestroy() const { return _markToDestroy; }
 
-bool handleInput(const std::vector& payload);
-
 private:
 
 /// Sends the .uno:Save command to LoKit.
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 1266417..5de9afc 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -533,7 +533,6 @@ private:
 // Set one we just created.
 Log::debug("New DocumentBroker for docKey [" + docKey + "].");
 docBroker = std::make_shared(uriPublic, docKey, 
LOOLWSD::ChildRoot, child);
-child->setDocumentBroker(docBroker);
 }
 
 // Validate the broker.
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-05-01 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   30 ++
 loolwsd/DocumentBroker.hpp |   20 +++-
 loolwsd/LOOLWSD.cpp|1 +
 3 files changed, 50 insertions(+), 1 deletion(-)

New commits:
commit 10417c9447ec1d34a8a599daf28ac4339a37930a
Author: Ashod Nakashian 
Date:   Sun May 1 20:39:36 2016 -0400

loolwsd: establish communication with child from DocumentBroker

The WebSocket that each child created with WSD is not used
except to request the child to load the document a client
requests. Beyond this point, it was not utilized for anything.

In fact, there are no handlers in WSD for messages coming
from the child; it is a one-way communication.

That is until now. With the move to unify communication
between WSD and each child, DocumentBroker can now
receive and handle messages from its ChildProcess.

Change-Id: Ie7f030a92db8303cd7087fff2325f136a49bc7fc
Reviewed-on: https://gerrit.libreoffice.org/24581
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 8f50e32..52e30f0 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -17,6 +17,27 @@
 #include "LOOLWSD.hpp"
 #include "Storage.hpp"
 #include "TileCache.hpp"
+#include "LOOLProtocol.hpp"
+
+using namespace LOOLProtocol;
+
+void ChildProcess::socketProcessor()
+{
+IoUtil::SocketProcessor(_ws,
+[this](const std::vector& payload)
+{
+auto docBroker = this->_docBroker.lock();
+if (docBroker)
+{
+return docBroker->handleInput(payload);
+}
+
+Log::warn("No DocumentBroker to handle child message: [" + 
LOOLProtocol::getAbbreviatedMessage(payload) + "].");
+return true;
+},
+[]() { },
+[this]() { return !!this->_stop; });
+}
 
 namespace
 {
@@ -79,6 +100,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
+
 Log::info("DocumentBroker [" + _uriPublic.toString() + "] created. DocKey: 
[" + _docKey + "]");
 }
 
@@ -359,6 +381,14 @@ size_t DocumentBroker::removeSession(const std::string& id)
 return _sessions.size();
 }
 
+bool DocumentBroker::handleInput(const std::vector& payload)
+{
+Log::trace("DocumentBroker got child message: [" + 
LOOLProtocol::getAbbreviatedMessage(payload) + "].");
+
+//TODO: Handle message.
+return true;
+}
+
 bool DocumentBroker::canDestroy()
 {
 std::unique_lock lock(_mutex);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 965ced3..efdedc3 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -39,8 +40,10 @@ public:
 /// @param ws is the control WebSocket to the child.
 ChildProcess(const Poco::Process::PID pid, const 
std::shared_ptr& ws) :
 _pid(pid),
-_ws(ws)
+_ws(ws),
+_stop(false)
 {
+_thread = std::thread([this]() { this->socketProcessor(); });
 Log::info("ChildProcess ctor [" + std::to_string(_pid) + "].");
 }
 
@@ -57,8 +60,15 @@ public:
 }
 }
 
+void setDocumentBroker(const std::shared_ptr& docBroker)
+{
+_docBroker = docBroker;
+}
+
 void close(const bool rude)
 {
+_stop = true;
+_thread.join();
 _ws.reset();
 if (_pid != -1)
 {
@@ -95,8 +105,14 @@ public:
 }
 
 private:
+void socketProcessor();
+
+private:
 Poco::Process::PID _pid;
 std::shared_ptr _ws;
+std::weak_ptr _docBroker;
+std::thread _thread;
+std::atomic _stop;
 };
 
 /// DocumentBroker is responsible for setting up a document
@@ -184,6 +200,8 @@ public:
 bool canDestroy();
 bool isMarkedToDestroy() const { return _markToDestroy; }
 
+bool handleInput(const std::vector& payload);
+
 private:
 
 /// Sends the .uno:Save command to LoKit.
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 5de9afc..1266417 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -533,6 +533,7 @@ private:
 // Set one we just created.
 Log::debug("New DocumentBroker for docKey [" + docKey + "].");
 docBroker = std::make_shared(uriPublic, docKey, 
LOOLWSD::ChildRoot, child);
+child->setDocumentBroker(docBroker);
 }
 
 // Validate the broker.
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-04-24 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   17 +++
 loolwsd/DocumentBroker.hpp |2 
 loolwsd/LOOLWSD.cpp|  106 -
 3 files changed, 48 insertions(+), 77 deletions(-)

New commits:
commit 4f7b911066bd29d5901b2724b85aae77258b73eb
Author: Ashod Nakashian 
Date:   Sun Apr 24 22:09:13 2016 -0400

loolwsd: simplified the bridging between client and prisoner sessions

Change-Id: I1335060963eda3356312f42060da229f43d239d8
Reviewed-on: https://gerrit.libreoffice.org/24358
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 31de15e..fc7d887 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -292,6 +292,23 @@ size_t 
DocumentBroker::addSession(std::shared_ptr& session
 return _sessions.size();
 }
 
+bool DocumentBroker::connectPeers(std::shared_ptr& 
session)
+{
+const auto id = session->getId();
+
+std::lock_guard lock(_mutex);
+
+auto it = _sessions.find(id);
+if (it != _sessions.end())
+{
+it->second->setPeer(session);
+session->setPeer(it->second);
+return true;
+}
+
+return false;
+}
+
 size_t DocumentBroker::removeSession(const std::string& id)
 {
 std::lock_guard lock(_mutex);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index dcf2d2a..5a5298f 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -194,6 +194,8 @@ public:
 
 /// Add a new session. Returns the new number of sessions.
 size_t addSession(std::shared_ptr& session);
+/// Connect a prison session to its client peer.
+bool connectPeers(std::shared_ptr& session);
 /// Removes a session by ID. Returns the new number of sessions.
 size_t removeSession(const std::string& id);
 
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 675444e..068f25c 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -271,51 +271,28 @@ public:
 class ClientRequestHandler: public HTTPRequestHandler
 {
 private:
-
-static bool waitBridgeCompleted(const 
std::shared_ptr& clientSession,
-const std::shared_ptr& 
docBroker)
+static void waitBridgeCompleted(const 
std::shared_ptr& session)
 {
-int retries = 5;
 bool isFound = false;
-
-// Wait until the client has connected with a prison socket.
-std::shared_ptr prisonSession;
 std::unique_lock lock(AvailableChildSessionMutex);
+Log::debug() << "Waiting for client session [" << session->getId() << 
"] to connect." << Log::end;
+AvailableChildSessionCV.wait_for(
+lock,
+std::chrono::milliseconds(COMMAND_TIMEOUT_MS),
+[, ]
+{
+return (isFound = 
AvailableChildSessions.find(session->getId()) != AvailableChildSessions.end());
+});
 
-Log::debug() << "Waiting for client session [" << 
clientSession->getId() << "] to connect." << Log::end;
-while (!TerminationFlag && retries-- && !isFound)
-{
-AvailableChildSessionCV.wait_for(
-lock,
-std::chrono::milliseconds(COMMAND_TIMEOUT_MS),
-[, ]
-{
-return (isFound = 
AvailableChildSessions.find(clientSession->getId()) != 
AvailableChildSessions.end());
-});
-
-if (!isFound)
-{
-//FIXME: outdated!
-Log::info() << "Retrying client permission... " << retries 
<< Log::end;
-// request again new URL session
-const std::string message = "request " + 
clientSession->getId() + " " + docBroker->getDocKey() + '\n';
-Log::trace("MasterToBroker: " + message.substr(0, 
message.length()-1));
-IoUtil::writeFIFO(LOOLWSD::ForKitWritePipe, message);
-}
-}
-
-if (isFound)
+if (!isFound)
 {
-Log::debug("Waiting child session permission, done!");
-prisonSession = AvailableChildSessions[clientSession->getId()];
-AvailableChildSessions.erase(clientSession->getId());
-
-clientSession->setPeer(prisonSession);
-prisonSession->setPeer(clientSession);
-Log::debug("Connected " + clientSession->getName() + " - " + 
prisonSession->getName() + ".");
+// Let the client know we can't serve now.
+Log::error(session->getName() + ": Failed to connect to lokit 
process. Client cannot serve now.");
+throw 
WebSocketErrorMessageException(SERVICE_UNAVALABLE_INTERNAL_ERROR);
 }
 
-return isFound;
+Log::debug("Waiting child session permission, done!");
+AvailableChildSessions.erase(session->getId());

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/MasterProcessSession.cpp

2016-04-21 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp   |4 +++-
 loolwsd/DocumentBroker.hpp   |3 +++
 loolwsd/LOOLWSD.cpp  |5 -
 loolwsd/MasterProcessSession.cpp |   11 +++
 4 files changed, 21 insertions(+), 2 deletions(-)

New commits:
commit bde67c99344751702116e3409d96b52db07b15e8
Author: Ashod Nakashian 
Date:   Fri Apr 22 00:11:24 2016 -0400

loolwsd: track document modified state to avoid unnecessary auto-saving

This also avoids the feedback loop that results from the kit
thinking the previously inactive client is now active and
sending commands (.uno:Save).

Change-Id: I47074b35a922da15592d550032d494ba1efab83e
Reviewed-on: https://gerrit.libreoffice.org/24287
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 10776b3..5a21bad 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -73,7 +73,8 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _cacheRoot(getCachePath(uriPublic.toString())),
 _lastSaveTime(std::chrono::steady_clock::now()),
 _childProcess(childProcess),
-_markToDestroy(false)
+_markToDestroy(false),
+_isModified(false)
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
@@ -153,6 +154,7 @@ bool DocumentBroker::save()
 assert(_storage && _tileCache);
 if (_storage->saveLocalFileToStorage())
 {
+_isModified = false;
 _lastSaveTime = std::chrono::steady_clock::now();
 _tileCache->documentSaved();
 Log::debug("Saved to URI [" + uri + "] and updated tile cache.");
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 2a1e73d..b696a65 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -177,6 +177,8 @@ public:
 // Called when the last view is going out.
 bool canDestroy();
 bool isMarkedToDestroy() const { return _markToDestroy; }
+bool isModified() const { return _isModified; }
+void setModified(const bool value) { _isModified = value; }
 
 private:
 const Poco::URI _uriPublic;
@@ -192,6 +194,7 @@ private:
 std::unique_ptr _tileCache;
 std::shared_ptr _childProcess;
 bool _markToDestroy;
+bool _isModified;
 mutable std::mutex _mutex;
 std::condition_variable _saveCV;
 std::mutex _saveMutex;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index e6886a4..6a377fc 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -1594,7 +1594,10 @@ int LOOLWSD::main(const std::vector& 
/*args*/)
 std::unique_lock 
docBrokersLock(docBrokersMutex);
 for (auto& brokerIt : docBrokers)
 {
-brokerIt.second->autoSave(false);
+if (brokerIt.second->isModified())
+{
+brokerIt.second->autoSave(false);
+}
 }
 }
 catch (const std::exception& exc)
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index 1fa0a03..99f143e 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -164,6 +164,17 @@ bool MasterProcessSession::_handleInput(const char 
*buffer, int length)
 
 return true;
 }
+else if (tokens.count() == 2 && tokens[0] == "statechanged:")
+{
+StringTokenizer stateTokens(tokens[1], "=", 
StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
+if (stateTokens.count() == 2 && stateTokens[0] == 
".uno:ModifiedStatus")
+{
+if (_docBroker)
+{
+_docBroker->setModified(stateTokens[1] == "true");
+}
+}
+}
 }
 
 if (peer && !_isDocPasswordProtected)
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-04-17 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   22 +-
 loolwsd/DocumentBroker.hpp |5 -
 loolwsd/LOOLWSD.cpp|   33 +++--
 3 files changed, 56 insertions(+), 4 deletions(-)

New commits:
commit b933988a5976e1a621dde7f4d2b0f57b34cd58cd
Author: Ashod Nakashian 
Date:   Sun Apr 17 23:29:03 2016 -0400

loolwsd: flag and wait if document is unloading before relaoding

When a new view is created on a document that is
in the process of unloading, all sorts of things
can go wrong. This is especially problematic when
the document needs to be saved before unloading,
which takes significantly longer than otherwise.

Change-Id: Ib33a18cafa9d5a3a17f6bd8c6145f9331ae54044
Reviewed-on: https://gerrit.libreoffice.org/24184
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 7fdb7b3..75f3440 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -72,7 +72,8 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _childRoot(childRoot),
 _cacheRoot(getCachePath(uriPublic.toString())),
 _lastSaveTime(std::chrono::steady_clock::now()),
-_childProcess(childProcess)
+_childProcess(childProcess),
+_markToDestroy(false)
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
@@ -102,6 +103,12 @@ bool DocumentBroker::load(const std::string& jailId)
 
 std::unique_lock lock(_mutex);
 
+if (_markToDestroy)
+{
+// Tearing down.
+return false;
+}
+
 if (_storage)
 {
 // Already loaded. Nothing to do.
@@ -307,4 +314,17 @@ size_t DocumentBroker::removeSession(const std::string& id)
 return _sessions.size();
 }
 
+bool DocumentBroker::canDestroy()
+{
+std::unique_lock lock(_mutex);
+
+if (_sessions.size() == 1)
+{
+// Last view going away, can destroy.
+_markToDestroy = true;
+}
+
+return _markToDestroy;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 260a78d..2a1e73d 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -174,7 +174,9 @@ public:
 /// Removes a session by ID. Returns the new number of sessions.
 size_t removeSession(const std::string& id);
 
-void kill() { _childProcess->close(true); };
+// Called when the last view is going out.
+bool canDestroy();
+bool isMarkedToDestroy() const { return _markToDestroy; }
 
 private:
 const Poco::URI _uriPublic;
@@ -189,6 +191,7 @@ private:
 std::unique_ptr _storage;
 std::unique_ptr _tileCache;
 std::shared_ptr _childProcess;
+bool _markToDestroy;
 mutable std::mutex _mutex;
 std::condition_variable _saveCV;
 std::mutex _saveMutex;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 7f833fb..0313a1a 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -494,8 +494,33 @@ private:
 Log::debug("Found DocumentBroker for docKey [" + docKey + "].");
 docBroker = it->second;
 assert(docBroker);
+
+// If this document is going out, wait.
+if (docBroker->isMarkedToDestroy())
+{
+Log::debug("Document [" + docKey + "] is marked to destroy, 
waiting to load.");
+const auto timeout = POLL_TIMEOUT_MS / 2;
+for (size_t i = 0; i < COMMAND_TIMEOUT_MS / timeout; ++i)
+{
+docBrokersLock.unlock();
+
std::this_thread::sleep_for(std::chrono::milliseconds(timeout));
+docBrokersLock.lock();
+if (docBrokers.find(docKey) == docBrokers.end())
+{
+docBroker.reset();
+break;
+}
+}
+
+if (docBroker)
+{
+// Still here, but marked to destroy.
+throw std::runtime_error("Cannot load a view to document 
while unloading.");
+}
+}
 }
-else
+
+if (!docBroker)
 {
 // Request a kit process for this doc.
 auto child = getNewChild();
@@ -554,7 +579,11 @@ private:
 },
 []() { return TerminationFlag; });
 
-if (docBroker->getSessionsCount() == 1 && !session->_bLoadError)
+docBrokersLock.lock();
+const bool canDestroy = docBroker->canDestroy();
+docBrokersLock.unlock();
+
+if (canDestroy && !session->_bLoadError)
 {
 Log::info("Shutdown of the last session, saving the document 
before tearing down.");
 
___
Libreoffice-commits mailing list

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-04-17 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   59 -
 loolwsd/DocumentBroker.hpp |   20 ++-
 loolwsd/LOOLWSD.cpp|   22 ++--
 3 files changed, 50 insertions(+), 51 deletions(-)

New commits:
commit 372baaf427805f8cad79f3ef817c63d33ae20ce3
Author: Ashod Nakashian 
Date:   Sat Apr 16 17:18:51 2016 -0400

loolwsd: cleanup of DocumentBroker session management

Improved session add/remove API.
Reduced mutex count.
Renamed members and variables.
Added documentation.

Change-Id: If15991971484d4d508714c9436a51b291f42079f
Reviewed-on: https://gerrit.libreoffice.org/24158
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 0274cf0..85a8d17 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -151,8 +151,8 @@ bool DocumentBroker::save()
 
 bool DocumentBroker::autoSave(const bool force)
 {
-std::unique_lock sessionsLock(_wsSessionsMutex);
-if (_wsSessions.empty())
+std::unique_lock lock(_mutex);
+if (_sessions.empty())
 {
 // Shouldn't happen.
 return false;
@@ -160,7 +160,7 @@ bool DocumentBroker::autoSave(const bool force)
 
 // Find the most recent activity.
 double inactivityTimeMs = std::numeric_limits::max();
-for (auto& sessionIt: _wsSessions)
+for (auto& sessionIt: _sessions)
 {
 inactivityTimeMs = std::min(sessionIt.second->getInactivityMS(), 
inactivityTimeMs);
 }
@@ -182,7 +182,7 @@ bool DocumentBroker::autoSave(const bool force)
 
 // Save using session holding the edit-lock
 bool sent = false;
-for (auto& sessionIt: _wsSessions)
+for (auto& sessionIt: _sessions)
 {
 if (!sessionIt.second->isEditLocked())
 continue;
@@ -231,8 +231,8 @@ std::string DocumentBroker::getJailRoot() const
 
 void DocumentBroker::takeEditLock(const std::string& id)
 {
-std::lock_guard sessionsLock(_wsSessionsMutex);
-for (auto& it: _wsSessions)
+std::lock_guard lock(_mutex);
+for (auto& it: _sessions)
 {
 if (it.first != id)
 {
@@ -247,51 +247,56 @@ void DocumentBroker::takeEditLock(const std::string& id)
 }
 }
 
-void DocumentBroker::addWSSession(const std::string& id, 
std::shared_ptr& ws)
+size_t DocumentBroker::addSession(std::shared_ptr& 
session)
 {
-std::lock_guard sessionsLock(_wsSessionsMutex);
+const auto id = session->getId();
+
+std::lock_guard lock(_mutex);
 
-auto ret = _wsSessions.emplace(id, ws);
+auto ret = _sessions.emplace(id, session);
 if (!ret.second)
 {
 Log::warn("DocumentBroker: Trying to add already existed session.");
 }
 
-if (_wsSessions.size() == 1)
+if (_sessions.size() == 1)
 {
-ws->setEditLock(true);
-ws->sendTextFrame("editlock: 1");
+session->setEditLock(true);
+session->sendTextFrame("editlock: 1");
 }
 
 // Request a new session from the child kit.
 const std::string aMessage = "session " + id + " " + _docKey + "\n";
 Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() - 
1));
 _childProcess->getWebSocket()->sendFrame(aMessage.data(), aMessage.size());
+
+return _sessions.size();
 }
 
-void DocumentBroker::removeWSSession(const std::string& id)
+size_t DocumentBroker::removeSession(const std::string& id)
 {
-std::lock_guard sessionsLock(_wsSessionsMutex);
+std::lock_guard lock(_mutex);
 
-bool haveEditLock = false;
-auto it = _wsSessions.find(id);
-if (it != _wsSessions.end())
+auto it = _sessions.find(id);
+if (it != _sessions.end())
 {
-haveEditLock = it->second->isEditLocked();
+const auto haveEditLock = it->second->isEditLocked();
 it->second->setEditLock(false);
-_wsSessions.erase(it);
-}
+_sessions.erase(it);
 
-if (haveEditLock)
-{
-// pass the edit lock to first session in map
-it = _wsSessions.begin();
-if (it != _wsSessions.end())
+if (haveEditLock)
 {
-it->second->setEditLock(true);
-it->second->sendTextFrame("editlock: 1");
+// pass the edit lock to first session in map
+it = _sessions.begin();
+if (it != _sessions.end())
+{
+it->second->setEditLock(true);
+it->second->sendTextFrame("editlock: 1");
+}
 }
 }
+
+return _sessions.size();
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 599fb3f..df8287a 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -147,10 +147,10 @@ public:
 const std::string& getDocKey() const

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-04-10 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   30 +++---
 loolwsd/DocumentBroker.hpp |   13 -
 loolwsd/LOOLWSD.cpp|   16 
 3 files changed, 51 insertions(+), 8 deletions(-)

New commits:
commit 280669c253af08c001238b9465d6fa4e048cb545
Author: Ashod Nakashian 
Date:   Sun Apr 10 22:07:09 2016 -0400

loolwsd: save on disconnection

Last client disconnection now correctly issues a save
and waits for the confirmation before tearing down
the sockets, queues and threads.

Change-Id: I28c28d79a17d359e9aa1fe67b983ca9fb592b847
Reviewed-on: https://gerrit.libreoffice.org/23978
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index b2d93f1..c7caba4 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -130,6 +130,8 @@ bool DocumentBroker::load(const std::string& jailId)
 
 bool DocumentBroker::save()
 {
+std::unique_lock lock(_saveMutex);
+
 const auto uri = _uriPublic.toString();
 Log::debug("Saving to URI [" + uri + "].");
 
@@ -139,6 +141,7 @@ bool DocumentBroker::save()
 _lastSaveTime = std::chrono::steady_clock::now();
 _tileCache->documentSaved();
 Log::debug("Saved to URI [" + uri + "] and updated tile cache.");
+_saveCV.notify_all();
 return true;
 }
 
@@ -146,13 +149,13 @@ bool DocumentBroker::save()
 return false;
 }
 
-void DocumentBroker::autoSave()
+bool DocumentBroker::autoSave(const bool force)
 {
 std::unique_lock sessionsLock(_wsSessionsMutex);
 if (_wsSessions.empty())
 {
 // Shouldn't happen.
-return;
+return false;
 }
 
 // Find the most recent activity.
@@ -170,8 +173,10 @@ void DocumentBroker::autoSave()
 if (inactivityTimeMs < timeSinceLastSaveMs)
 {
 // Either we've been idle long enough, or it's auto-save time.
+// Or we are asked to save anyway.
 if (inactivityTimeMs >= IdleSaveDurationMs ||
-timeSinceLastSaveMs >= AutoSaveDurationMs)
+timeSinceLastSaveMs >= AutoSaveDurationMs ||
+force)
 {
 Log::info("Auto-save triggered for doc [" + _docKey + "].");
 
@@ -192,8 +197,27 @@ void DocumentBroker::autoSave()
 {
 Log::error("Failed to auto-save doc [" + _docKey + "]: No 
valid sessions.");
 }
+
+return sent;
 }
 }
+
+return false;
+}
+
+bool DocumentBroker::waitSave(const size_t timeoutMs)
+{
+std::unique_lock lock(_saveMutex);
+
+// Remeber the last save time, since this is the predicate.
+const auto lastSaveTime = _lastSaveTime;
+
+if (_saveCV.wait_for(lock, std::chrono::milliseconds(timeoutMs)) == 
std::cv_status::no_timeout)
+{
+return true;
+}
+
+return (lastSaveTime != _lastSaveTime);
 }
 
 std::string DocumentBroker::getJailRoot() const
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 17a1cfe..27a527a 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -13,6 +13,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -131,7 +132,15 @@ public:
 
 bool save();
 
-void autoSave();
+/// Save the document if there was activity since last save.
+/// force when true, will force saving immediatly, regardless
+/// of how long ago the activity was.
+bool autoSave(const bool force);
+
+/// Wait until the document is saved next.
+/// This is used to cleanup after the last save.
+/// Returns false if times out.
+bool waitSave(const size_t timeoutMs);
 
 Poco::URI getPublicUri() const { return _uriPublic; }
 Poco::URI getJailedUri() const { return _uriJailed; }
@@ -178,6 +187,8 @@ private:
 std::unique_ptr _tileCache;
 std::shared_ptr _childProcess;
 std::mutex _mutex;
+std::condition_variable _saveCV;
+std::mutex _saveMutex;
 
 static constexpr auto IdleSaveDurationMs = 30 * 1000;
 static constexpr auto AutoSaveDurationMs = 300 * 1000;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 989c3f7..0f5a391 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -592,10 +592,18 @@ private:
 
 if (docBroker->getSessionsCount() == 1 && !normalShutdown && 
!session->_bLoadError)
 {
-//TODO: This isn't this simple. We need to wait for the 
notification
-// of save so Storage can persist the save (if necessary).
 Log::info("Non-deliberate shutdown of the last session, saving the 
document before tearing down.");
-queue->put("uno .uno:Save");
+
+// Use auto-save to save only when there are modifications since 
last save.
+// We also need to wait until the save notification reaches us
+// and Storage persists

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp

2016-04-10 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   13 -
 loolwsd/DocumentBroker.hpp |   15 ---
 loolwsd/LOOLWSD.cpp|   24 
 3 files changed, 28 insertions(+), 24 deletions(-)

New commits:
commit 60531e7def7ed8be8caddf823449cc3ff53da20a
Author: Ashod Nakashian 
Date:   Sun Apr 10 13:03:57 2016 -0400

loolwsd: removed redundant session count in DocumentBroker

The sessions container already has the number of sessions.
No need for separate counters to track them.

Change-Id: I838865e2b8a843e87e81a6cc1226bcacd774b032
Reviewed-on: https://gerrit.libreoffice.org/23964
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 3df2254..4551270 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -71,8 +71,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _childRoot(childRoot),
 _cacheRoot(getCachePath(uriPublic.toString())),
 _lastSaveTime(std::chrono::steady_clock::now()),
-_childProcess(childProcess),
-_sessionsCount(0)
+_childProcess(childProcess)
 {
 assert(!_docKey.empty());
 assert(!_childRoot.empty());
@@ -125,22 +124,25 @@ bool DocumentBroker::load(const std::string& jailId)
 
 return true;
 }
-else
-return false;
+
+return false;
 }
 
 bool DocumentBroker::save()
 {
-Log::debug("Saving to URI: " + _uriPublic.toString());
+const auto uri = _uriPublic.toString();
+Log::debug("Saving to URI [" + uri + "].");
 
 assert(_storage && _tileCache);
 if (_storage->saveLocalFileToStorage())
 {
 _lastSaveTime = std::chrono::steady_clock::now();
 _tileCache->documentSaved();
+Log::debug("Saved to URI [" + uri + "] and updated tile cache.");
 return true;
 }
 
+Log::error("Failed to save to URI [" + uri + "].");
 return false;
 }
 
@@ -221,6 +223,7 @@ void DocumentBroker::takeEditLock(const std::string id)
 void DocumentBroker::addWSSession(const std::string id, 
std::shared_ptr& ws)
 {
 std::lock_guard sessionsLock(_wsSessionsMutex);
+
 auto ret = _wsSessions.emplace(id, ws);
 if (!ret.second)
 {
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 259572f..17a1cfe 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -120,8 +120,8 @@ public:
 ~DocumentBroker()
 {
 Log::info() << "~DocumentBroker [" << _uriPublic.toString()
-<< "] destroyed with " << _sessionsCount
-<< " sessions." << Log::end;
+<< "] destroyed with " << getSessionsCount()
+<< " sessions left." << Log::end;
 }
 
 void validate(const Poco::URI& uri);
@@ -137,10 +137,12 @@ public:
 Poco::URI getJailedUri() const { return _uriJailed; }
 const std::string& getJailId() const { return _jailId; }
 const std::string& getDocKey() const { return _docKey; }
-unsigned decSessions() { return --_sessionsCount; }
-unsigned incSessions() { return ++_sessionsCount; }
-unsigned getSessionsCount() { return _sessionsCount; }
 TileCache& tileCache() { return *_tileCache; }
+unsigned getSessionsCount() const
+{
+std::lock_guard sessionsLock(_wsSessionsMutex);
+return _wsSessions.size();
+}
 
 /// Returns the time in milliseconds since last save.
 double getTimeSinceLastSaveMs() const
@@ -171,12 +173,11 @@ private:
 std::string _filename;
 std::chrono::steady_clock::time_point _lastSaveTime;
 std::map _wsSessions;
-std::mutex _wsSessionsMutex;
+mutable std::mutex _wsSessionsMutex;
 std::unique_ptr _storage;
 std::unique_ptr _tileCache;
 std::shared_ptr _childProcess;
 std::mutex _mutex;
-std::atomic _sessionsCount;
 
 static constexpr auto IdleSaveDurationMs = 30 * 1000;
 static constexpr auto AutoSaveDurationMs = 300 * 1000;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 0ff7e5d..823775d 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -341,10 +341,9 @@ private:
 std::shared_ptr ws;
 auto session = std::make_shared(id, 
LOOLSession::Kind::ToClient, ws, docBroker, nullptr);
 docBroker->addWSSession(id, session);
-unsigned wsSessionsCount = docBroker->getWSSessionsCount();
+auto wsSessionsCount = docBroker->getWSSessionsCount();
 Log::trace(docKey + ", ws_sessions++: " + 
std::to_string(wsSessionsCount));
 session->setEditLock(true);
-docBroker->incSessions();
 lock.unlock();
 
 if (!waitBridgeCompleted(session, docBroker))
@@ -384,7 +383,9 @@ private:

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/MasterProcessSession.cpp loolwsd/MasterProcessSession.hpp

2016-04-09 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp   |   37 +++
 loolwsd/DocumentBroker.hpp   |   13 +
 loolwsd/LOOLWSD.cpp  |   52 ++-
 loolwsd/MasterProcessSession.cpp |1 
 loolwsd/MasterProcessSession.hpp |1 
 5 files changed, 59 insertions(+), 45 deletions(-)

New commits:
commit e6d0882791fbf1b25b6d63e86f65c1f5224f2c4a
Author: Ashod Nakashian 
Date:   Sat Apr 9 23:20:20 2016 -0400

loolwsd: moved autosave to DocumentBroker

Autosaving is done by DocumentBroker, which
tracks the last save time.

There are two triggers: idle and auto save.
The first triggers when sufficient time passes
after the last interaction the user had with
the UI (currently 30 seconds).
The second triggers when it's been more than
5 minutes since the last save.
Both triggers are conditional on the user
being active after the last save.

The new code auto-saves doesn't issue
a save command per session, but only
one per doc.

Change-Id: Iada15c16002e70710d2c13a3dcfdab036d8935c6
Reviewed-on: https://gerrit.libreoffice.org/23951
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index aee583d..862b298 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -70,6 +70,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _docKey(docKey),
 _childRoot(childRoot),
 _cacheRoot(getCachePath(uriPublic.toString())),
+_lastSaveTime(std::chrono::steady_clock::now()),
 _childProcess(childProcess),
 _sessionsCount(0)
 {
@@ -135,6 +136,7 @@ bool DocumentBroker::save()
 assert(_storage && _tileCache);
 if (_storage->saveLocalFileToStorage())
 {
+_lastSaveTime = std::chrono::steady_clock::now();
 _tileCache->documentSaved();
 return true;
 }
@@ -142,6 +144,41 @@ bool DocumentBroker::save()
 return false;
 }
 
+void DocumentBroker::autoSave()
+{
+std::unique_lock sessionsLock(_wsSessionsMutex);
+if (_wsSessions.empty())
+{
+// Shouldn't happen.
+return;
+}
+
+// Find the most recent activity.
+double inactivityTimeMs = std::numeric_limits::max();
+for (auto& sessionIt: _wsSessions)
+{
+inactivityTimeMs = std::min(sessionIt.second->getInactivityMS(), 
inactivityTimeMs);
+}
+
+Log::trace("Most recent inactivity was " + 
std::to_string(inactivityTimeMs) + " ms ago.");
+const auto timeSinceLastSaveMs = getTimeSinceLastSaveMs();
+Log::trace("Time since last save was " + 
std::to_string(timeSinceLastSaveMs) + " ms ago.");
+
+// There has been some editing since we saved last?
+if (inactivityTimeMs < timeSinceLastSaveMs)
+{
+// Either we've been idle long enough, or it's auto-save time.
+if (inactivityTimeMs >= IdleSaveDurationMs ||
+timeSinceLastSaveMs >= AutoSaveDurationMs)
+{
+Log::info("Auto-save triggered for doc [" + _docKey + "].");
+
+// Any session can be used to save.
+_wsSessions.begin()->second->getQueue()->put("uno .uno:Save");
+}
+}
+}
+
 std::string DocumentBroker::getJailRoot() const
 {
 assert(!_jailId.empty());
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 7dedb64..66e2497 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -131,6 +131,8 @@ public:
 
 bool save();
 
+void autoSave();
+
 Poco::URI getPublicUri() const { return _uriPublic; }
 Poco::URI getJailedUri() const { return _uriJailed; }
 const std::string& getJailId() const { return _jailId; }
@@ -140,6 +142,13 @@ public:
 unsigned getSessionsCount() { return _sessionsCount; }
 TileCache& tileCache() { return *_tileCache; }
 
+/// Returns the time in milliseconds since last save.
+double getTimeSinceLastSaveMs() const
+{
+const auto duration = (std::chrono::steady_clock::now() - 
_lastSaveTime);
+return 
std::chrono::duration_cast(duration).count();
+}
+
 std::string getJailRoot() const;
 
 /// Ignore input events from all web socket sessions
@@ -164,11 +173,15 @@ private:
 Poco::URI _uriJailed;
 std::string _jailId;
 std::string _filename;
+std::chrono::steady_clock::time_point _lastSaveTime;
 std::unique_ptr _storage;
 std::unique_ptr _tileCache;
 std::shared_ptr _childProcess;
 std::mutex _mutex;
 std::atomic _sessionsCount;
+
+static constexpr auto IdleSaveDurationMs = 30 * 1000;
+static constexpr auto AutoSaveDurationMs = 300 * 1000;
 };
 
 #endif
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index a1a3b2a..9e32cbd 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -511,9 +511,6 @@ private:
 }

[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/TileCache.cpp loolwsd/TileCache.hpp

2016-03-26 Thread Ashod Nakashian 
 loolwsd/DocumentBroker.cpp |   21 -
 loolwsd/DocumentBroker.hpp |1 +
 loolwsd/LOOLWSD.cpp|1 -
 loolwsd/TileCache.cpp  |   29 +++--
 loolwsd/TileCache.hpp  |9 +++--
 5 files changed, 43 insertions(+), 18 deletions(-)

New commits:
commit 8b34e75722d4ca3a15d88322cebcd40dde45e5b2
Author: Ashod Nakashian 
Date:   Sat Mar 26 07:50:13 2016 -0400

loolwsd: cache directory path moved to DocumentBroker

Change-Id: Ic7733bf4f35243afeb34d0ac2d85b619b8f49457
Reviewed-on: https://gerrit.libreoffice.org/23533
Reviewed-by: Ashod Nakashian 
Tested-by: Ashod Nakashian 

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index d5af7f6..82dc71f 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -8,11 +8,29 @@
  */
 
 #include 
+#include 
 
+#include "LOOLWSD.hpp"
 #include "DocumentBroker.hpp"
 #include "Storage.hpp"
 #include "TileCache.hpp"
 
+namespace
+{
+
+/// Returns the cache path for a given document URI.
+std::string getCachePath(const std::string& uri)
+{
+Poco::SHA1Engine digestEngine;
+
+digestEngine.update(uri.c_str(), uri.size());
+
+return (LOOLWSD::Cache + "/" +
+Poco::DigestEngine::digestToHex(digestEngine.digest()).insert(3, 
"/").insert(2, "/").insert(1, "/"));
+}
+
+}
+
 Poco::URI DocumentBroker::sanitizeURI(std::string uri)
 {
 // The URI of the document should be url-encoded.
@@ -48,6 +66,7 @@ DocumentBroker::DocumentBroker(const Poco::URI& uriPublic,
 _uriPublic(uriPublic),
 _docKey(docKey),
 _childRoot(childRoot),
+_cacheRoot(getCachePath(uriPublic.toString())),
 _sessionsCount(0)
 {
 assert(!_docKey.empty());
@@ -86,7 +105,7 @@ bool DocumentBroker::load(const std::string& jailId)
 Log::info("jailPath: " + jailPath.toString() + ", jailRoot: " + jailRoot);
 
 const std::string timestamp = ""; //FIXME: Should come from load options.
-_tileCache.reset(new TileCache(_uriPublic.toString(), timestamp));
+_tileCache.reset(new TileCache(_uriPublic.toString(), timestamp, 
_cacheRoot));
 
 _storage = createStorage(jailRoot, jailPath.toString(), _uriPublic);
 
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 8871e39..c2fa43b 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -72,6 +72,7 @@ private:
 const Poco::URI _uriPublic;
 const std::string _docKey;
 const std::string _childRoot;
+const std::string _cacheRoot;
 Poco::URI _uriJailed;
 std::string _jailId;
 std::string _filename;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index b0dcde6..bec5a1d 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -150,7 +150,6 @@ using Poco::Net::WebSocketException;
 using Poco::Path;
 using Poco::Process;
 using Poco::ProcessHandle;
-using Poco::Random;
 using Poco::Runnable;
 using Poco::StreamCopier;
 using Poco::StringTokenizer;
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 120f985..5f2e70e 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -37,7 +36,6 @@
 using Poco::DigestEngine;
 using Poco::DirectoryIterator;
 using Poco::File;
-using Poco::SHA1Engine;
 using Poco::StringTokenizer;
 using Poco::SyntaxException;
 using Poco::Timestamp;
@@ -45,14 +43,25 @@ using Poco::URI;
 
 using namespace LOOLProtocol;
 
-TileCache::TileCache(const std::string& docURL, const std::string& timestamp) :
+TileCache::TileCache(const std::string& docURL,
+ const std::string& timestamp,
+ const std::string& rootCacheDir) :
 _docURL(docURL),
+_rootCacheDir(rootCacheDir),
+_persCacheDir(Poco::Path(rootCacheDir, "persistent").toString()),
+_editCacheDir(Poco::Path(rootCacheDir, "editing").toString()),
 _isEditing(false),
 _hasUnsavedChanges(false)
 {
+Log::info("TileCache ctor.");
 setup(timestamp);
 }
 
+TileCache::~TileCache()
+{
+Log::info("~TileCache dtor.");
+}
+
 std::unique_ptr TileCache::lookupTile(int part, int width, int 
height, int tilePosX, int tilePosY, int tileWidth, int tileHeight)
 {
 std::string cachedName = cacheFileName(part, width, height, tilePosX, 
tilePosY, tileWidth, tileHeight);
@@ -290,20 +299,12 @@ void TileCache::removeFile(const std::string fileName)
 
 std::string TileCache::toplevelCacheDirName()
 {
-SHA1Engine digestEngine;
-
-digestEngine.update(_docURL.c_str(), _docURL.size());
-
-return (LOOLWSD::Cache + "/" +
-DigestEngine::digestToHex(digestEngine.digest()).insert(3, 
"/").insert(2, "/").insert(1, "/"));
+return _rootCacheDir;
 }
 
-std::string TileCache::cacheDirName(bool useEditingCache)
+std::string TileCache::cacheDirName(const bool useEditingCache)
 {
-if (useEditingCache)
-