loolwsd/ChildProcessSession.cpp | 13 ------ loolwsd/ChildProcessSession.hpp | 2 - loolwsd/LOOLKit.cpp | 78 ++++++++++++++++++++++++++-------------- 3 files changed, 52 insertions(+), 41 deletions(-)
New commits: commit 9523769e94de520da1c528fd7e56baef44ff519f Author: Ashod Nakashian <[email protected]> Date: Fri Jan 8 22:31:34 2016 -0500 loolwsd: better callback handling and shutdown Change-Id: Id9cc9f748d2dac3afb7d7d002062f8c423bce775 Reviewed-on: https://gerrit.libreoffice.org/21321 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp index f3ef86d..0940509 100644 --- a/loolwsd/ChildProcessSession.cpp +++ b/loolwsd/ChildProcessSession.cpp @@ -38,7 +38,6 @@ using Poco::ProcessHandle; using Poco::StringTokenizer; using Poco::URI; -Poco::NotificationQueue ChildProcessSession::_callbackQueue; Poco::Mutex ChildProcessSession::_mutex; ChildProcessSession::ChildProcessSession(const std::string& id, @@ -69,20 +68,10 @@ ChildProcessSession::~ChildProcessSession() if (_loKitDocument != nullptr) { - if (_multiView) - _loKitDocument->pClass->setView(_loKitDocument, _viewId); - - _loKitDocument->pClass->registerCallback(_loKitDocument, nullptr, nullptr); - - if (_multiView) - _loKitDocument->pClass->destroyView(_loKitDocument, _viewId); + _onUnload(_viewId); Log::debug("Destroy view [" + getName() + "]-> [" + std::to_string(_viewId) + "]"); } - if (LIBREOFFICEKIT_HAS(_loKit, registerCallback)) - _loKit->pClass->registerCallback(_loKit, nullptr, nullptr); - - _onUnload(_viewId); } bool ChildProcessSession::_handleInput(const char *buffer, int length) diff --git a/loolwsd/ChildProcessSession.hpp b/loolwsd/ChildProcessSession.hpp index de3bc84..646e29c 100644 --- a/loolwsd/ChildProcessSession.hpp +++ b/loolwsd/ChildProcessSession.hpp @@ -50,8 +50,6 @@ public: LibreOfficeKitDocument *getLoKitDocument() const { return _loKitDocument; } - static Poco::NotificationQueue _callbackQueue; - protected: virtual bool loadDocument(const char *buffer, int length, Poco::StringTokenizer& tokens) override; diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp index dfef497..1564616 100644 --- a/loolwsd/LOOLKit.cpp +++ b/loolwsd/LOOLKit.cpp @@ -94,7 +94,8 @@ class CallBackWorker: public Runnable { public: CallBackWorker(NotificationQueue& queue): - _queue(queue) + _queue(queue), + _stop(false) { } @@ -117,7 +118,7 @@ public: case LOK_CALLBACK_GRAPHIC_SELECTION: return std::string("LOK_CALLBACK_GRAPHIC_SELECTION"); case LOK_CALLBACK_CELL_CURSOR: - return std::string("LLOK_CALLBACK_CELL_CURSOR"); + return std::string("LOK_CALLBACK_CELL_CURSOR"); case LOK_CALLBACK_CELL_FORMULA: return std::string("LOK_CALLBACK_CELL_FORMULA"); case LOK_CALLBACK_MOUSE_POINTER: @@ -269,7 +270,7 @@ public: #endif Log::debug("Thread [" + thread_name + "] started."); - while (!TerminationFlag) + while (!_stop && !TerminationFlag) { Notification::Ptr aNotification(_queue.waitDequeueNotification()); if (!TerminationFlag && aNotification) @@ -312,8 +313,14 @@ public: _queue.wakeUpAll(); } + void stop() + { + _stop = true; + } + private: NotificationQueue& _queue; + bool _stop; static FastMutex _mutex; }; @@ -429,6 +436,7 @@ public: queue.put("eof"); queueHandlerThread.join(); + // We should probably send the Client some sensible message and reason. _session->sendTextFrame("eof"); } catch (const Exception& exc) @@ -481,16 +489,29 @@ public: _url(url), _loKitDocument(nullptr), _clientViews(0), - _mainViewId(-1) + _callbackWorker(_callbackQueue) { Log::info("Document ctor for url [" + _url + "] on child [" + _jailId + "] LOK_VIEW_CALLBACK=" + std::to_string(_multiView) + "."); + + _callbackThread.start(_callbackWorker); } ~Document() { Log::info("~Document dtor for url [" + _url + "] on child [" + _jailId + - "]. There are " + std::to_string(_mainViewId) + " views."); + "]. There are " + std::to_string(_clientViews) + " views."); + + // Wait for the callback worker to finish. + _callbackWorker.stop(); + _callbackWorker.wakeUpAll(); + _callbackThread.join(); + + // Flag all connections to stop. + for (auto aIterator : _connections) + { + aIterator.second->stop(); + } // Destroy all connections and views. for (auto aIterator : _connections) @@ -505,8 +526,7 @@ public: else { // wait until loolwsd close all websockets - if (aIterator.second->isRunning()) - aIterator.second->join(); + aIterator.second->join(); } } @@ -517,8 +537,6 @@ public: // Destroy the document. if (_loKitDocument != nullptr) { - if (_multiView) - _loKitDocument->pClass->destroyView(_loKitDocument, _mainViewId); _loKitDocument->pClass->destroy(_loKitDocument); } } @@ -580,6 +598,12 @@ public: private: + static void ViewCallback(int nType, const char* pPayload, void* pData) + { + auto pNotif = new CallBackNotification(nType, pPayload ? pPayload : "(nil)", pData); + _callbackQueue.enqueueNotification(pNotif); + } + static void DocumentCallback(int nType, const char* pPayload, void* pData) { Document* self = reinterpret_cast<Document*>(pData); @@ -589,19 +613,12 @@ private: { if (it.second->isRunning()) { - auto pNotif = new CallBackNotification(nType, pPayload ? pPayload : "(nil)", it.second->getSession().get()); - ChildProcessSession::_callbackQueue.enqueueNotification(pNotif); + ViewCallback(nType, pPayload, it.second->getSession().get()); } } } } - static void ViewCallback(int nType, const char* pPayload, void* pData) - { - auto pNotif = new CallBackNotification(nType, pPayload ? pPayload : "(nil)", pData); - ChildProcessSession::_callbackQueue.enqueueNotification(pNotif); - } - /// Load a document (or view) and register callbacks. LibreOfficeKitDocument* onLoad(ChildProcessSession* session, const std::string& jailedFilePath) { @@ -641,10 +658,17 @@ private: void onUnload(const int viewId) { - --_clientViews; - Log::info() << "Document [" << _url << "] view [" - << viewId << "] unloaded, leaving " - << _clientViews << " views." << Log::end; + if (_multiView && _loKitDocument) + { + --_clientViews; + Log::info() << "Document [" << _url << "] view [" + << viewId << "] unloaded, leaving " + << _clientViews << " views." << Log::end; + + _loKitDocument->pClass->setView(_loKitDocument, viewId); + _loKitDocument->pClass->registerCallback(_loKitDocument, nullptr, nullptr); + _loKitDocument->pClass->destroyView(_loKitDocument, viewId); + } } private: @@ -659,9 +683,14 @@ private: std::mutex _mutex; std::map<std::string, std::shared_ptr<Connection>> _connections; std::atomic<unsigned> _clientViews; - int _mainViewId; + + CallBackWorker _callbackWorker; + Thread _callbackThread; + static Poco::NotificationQueue _callbackQueue; }; +Poco::NotificationQueue Document::_callbackQueue; + void lokit_main(const std::string &loSubPath, const std::string& jailId, const std::string& pipe) { #ifdef LOOLKIT_NO_MAIN @@ -720,9 +749,6 @@ void lokit_main(const std::string &loSubPath, const std::string& jailId, const s exit(-1); } - CallBackWorker callbackWorker(ChildProcessSession::_callbackQueue); - Poco::ThreadPool::defaultPool().start(callbackWorker); - Log::info("loolkit [" + std::to_string(Process::id()) + "] is ready."); std::string aResponse; @@ -813,8 +839,6 @@ void lokit_main(const std::string &loSubPath, const std::string& jailId, const s } } - // wait callback worker finish - callbackWorker.wakeUpAll(); Poco::ThreadPool::defaultPool().joinAll(); close(writerBroker); _______________________________________________ Libreoffice-commits mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
