[Libreoffice-commits] online.git: 4 commits - loleaflet/Makefile loleaflet/README loolwsd/LOOLBroker.cpp loolwsd/LOOLKit.cpp loolwsd/Makefile.am
Tor Lillqvist
tml at collabora.com
Mon Mar 7 12:21:24 UTC 2016
loleaflet/Makefile | 2 +-
loleaflet/README | 2 +-
loolwsd/LOOLBroker.cpp | 34 +++++-----------------------------
loolwsd/LOOLKit.cpp | 5 -----
loolwsd/Makefile.am | 11 +++++++----
5 files changed, 14 insertions(+), 40 deletions(-)
New commits:
commit 80c6a91d5d61af0d4a8b38531b0ef41ecebfdcdd
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Mar 7 13:08:02 2016 +0200
Don't call setcap on loolkit before we have built it
On the other hand, loolwsd does not need capabilities any more.
Also update the comment to match reality, and explain in more detail
what is going on.
diff --git a/loolwsd/Makefile.am b/loolwsd/Makefile.am
index abf3b7b..75c4a9f 100644
--- a/loolwsd/Makefile.am
+++ b/loolwsd/Makefile.am
@@ -39,10 +39,13 @@ clean-cache:
# Intentionally don't use "*" below... Avoid risk of accidentally running rm -rf /*
test -n "@LOOLWSD_CACHEDIR@" && rm -rf "@LOOLWSD_CACHEDIR@"/[0-9a-f]
-# After building loolwsd, set its capabilities to allow chroot(). Do
-# it already after a plain 'make' to allow for testing without
-# installing.
-all-local: loolwsd loolbroker
+# After building loolbroker and loolkit, set their capabilities as
+# required. Do it already after a plain 'make' to allow for testing
+# without installing. When building for packaging, no need for this,
+# as the capabilities won't survive packaging anyway. Instead, handle
+# it when installing the RPM or Debian package.
+
+all-local: loolbroker loolkit
if test "$$BUILDING_FROM_RPMBUILD" != yes; then \
sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep loolbroker; \
sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep loolkit; \
commit e96629b3716185a7fa10a61f786dc4ee3f007443
Author: Tor Lillqvist <tml at collabora.com>
Date: Mon Mar 7 12:59:14 2016 +0200
It is apparently intended that one uses the mocha built here, not a system one
On many/some distros, the system mocha is a much (?) older version,
which uses a wildly different output format, which is confusing.
diff --git a/loleaflet/Makefile b/loleaflet/Makefile
index 8228466..5742504 100644
--- a/loleaflet/Makefile
+++ b/loleaflet/Makefile
@@ -63,5 +63,5 @@ load-test: spec/data/load-test
mkdir load_test_out; \
for i in $$(seq 1 20); \
do \
- mocha spec/headlessLoadTest.js > load_test_out/$$i.out 2>&1 & \
+ node_modules/.bin/mocha spec/headlessLoadTest.js > load_test_out/$$i.out 2>&1 & \
done;
diff --git a/loleaflet/README b/loleaflet/README
index 6089569..f4ac2b7 100644
--- a/loleaflet/README
+++ b/loleaflet/README
@@ -61,7 +61,7 @@ Testing
- to simulate a client opening several documents in the browser
+ open spec/loadtest.html in the browser
- to simulate a client opening several documents in the console
- + run: mocha spec/headlessLoadTest.js
+ + run: node_modules/.bin/mocha spec/headlessLoadTest.js
- to simulate multiple clients opening several documents in the console
+ run: make load-test
commit 636fafa3b67d317909b5644cedb5d7950120020c
Author: Tor Lillqvist <tml at collabora.com>
Date: Thu Mar 3 17:09:36 2016 +0200
We use a recursive mutex, so no need to drop and re-take around documentLoad()
The callbacks from documentLoad() are made in the same thread.
Sure, as such it is not a good thing to use recursive mutexes. If we
switch back to non-recursive mutexes, we will have to stop taking the
lock in callbacks from documentLoad(), i.e. make sure we know those
functions aren't used elsewhere, in places where a lock would be
needed. Or something.
If a client session closes just after sending a load message to load a
document, and another session then fairly immediately connects and
sends a load message for the same document, the latter session gets
handled by the same kit process. Also, the same Document object is
apparently used. In that kit process, the first documentLoad() might
easily still be in progress. The handler for the new session still
calls onLoad(), too, and as the first onLoad() had dropped the lock
for the duration of the documentLoad() call, the new onLoad can take
the lock and call documentLoad(), too, while the first documentLoad()
call in the other thread still is in progress. This leads to
interesting problems.
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 9b3a66e..3550279 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -602,9 +602,6 @@ private:
LOK_FEATURE_DOCUMENT_PASSWORD_TO_MODIFY);
}
- // documentLoad will trigger callback, which needs to take the lock.
- lock.unlock();
-
// Save the provided password with us and the jailed url
_isDocPasswordProvided = isDocPasswordProvided;
_docPassword = docPassword;
@@ -634,8 +631,6 @@ private:
return nullptr;
}
Log::info("documentLoad() returned");
- // Retake the lock.
- lock.lock();
// Notify the Admin thread
std::ostringstream message;
commit 0194e8d57794e602995a02ee3c77d7905879de49
Author: Tor Lillqvist <tml at collabora.com>
Date: Fri Mar 4 16:21:29 2016 +0200
Use assert()
It is loolwsd that spawns loolbroker so we control what arguments it
gets, so no need to give verbose errors if our own code is
inconsistent. That is what assert() is for.
diff --git a/loolwsd/LOOLBroker.cpp b/loolwsd/LOOLBroker.cpp
index fe35be3..94005bf 100644
--- a/loolwsd/LOOLBroker.cpp
+++ b/loolwsd/LOOLBroker.cpp
@@ -634,35 +634,11 @@ int main(int argc, char** argv)
loolkitPath = Poco::Path(argv[0]).parent().toString() + "loolkit";
- if (loSubPath.empty())
- {
- Log::error("Error: --losubpath is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- if (sysTemplate.empty())
- {
- Log::error("Error: --systemplate is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- if (loTemplate.empty())
- {
- Log::error("Error: --lotemplate is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- if (childRoot.empty())
- {
- Log::error("Error: --childroot is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- if (numPreSpawnedChildren < 1)
- {
- Log::error("Error: --numprespawns is 0");
- std::exit(Application::EXIT_SOFTWARE);
- }
+ assert(!loSubPath.empty());
+ assert(!sysTemplate.empty());
+ assert(!loTemplate.empty());
+ assert(!childRoot.empty());
+ assert(numPreSpawnedChildren >= 1);
const Path pipePath = Path::forDirectory(childRoot + Path::separator() + FIFO_PATH);
const std::string pipeLoolwsd = Path(pipePath, FIFO_LOOLWSD).toString();
More information about the Libreoffice-commits
mailing list