It is not a good idea to have the same string somewhat arbitrarily
both as a static const members of the LOOLWSD class and then as a
file-local static const in another file. Or defined as a separate
local const static in each compilation unit that includes
Common.hpp. Use constexpr instead, in Common.hpp.
When the Document lock cannot be taken
purging doesn't block (which would block
the kit-broker pipe). Instead, purging
is done only when the lock is taken,
otherwise we try again later.
Change-Id: Id201f1c67803d9b1e765e8c55f85206795fe53c0
Reviewed-on: https://gerrit.libreoffice.org/23448
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Ideally, we will have a randomized path for the jails.
Unfortunately, this will make it harder to cleanup
after an ungraceful exit of a child, including recovery
of docs etc.
Having a PID for the jailId makes this issue easier by
implicitly implying the jail path for a given child.
To prevent security leaks, we should at least randomize
the doc directory within the jail, as such:
/chroot/<pid>/user/docs/<rand>/
For now we use jailId=pid=rand.
Change-Id: I948fba0aaef725c9c059780df0a184a86569d898
Reviewed-on: https://gerrit.libreoffice.org/23223
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The current format is more flexible and standard.
/loolwsd/child?sessionId=xxx&jailId=yyy
The sessionId is the client-specific connection ID (which
is originally passed to the child via the Broker.
The jailId is the PID of the child.
Change-Id: I69c88e84114f9678addf795896ca2da15ca1221b
Reviewed-on: https://gerrit.libreoffice.org/23211
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Much simpler.
Also, don't duplicate the code informing that LD_BIND_NOW or
LOK_VIEW_CALLBACK are not set. Not that I understand why we need to
inform about that? If the "normal" thing should be that they are set,
why don't make it so by default then?
I occasionally saw a deadlock when running 'make check' where one
thread holds the ChildProcessSession::Mutex and wants the _mutex for a
Document, while another thread holds that _mutex and wants the
Mutex. In particular, it is the Document::onUnload() that wants the
_mutex. So avoid the deadlock by having Document::onUnload() first
take the ChildProcessSession::Mutex.
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.
Admin web sessions are added as subscribers to AdminModel. Live
notification fill up the AdminModel, and notifies to
subscribers, if present any. AdminModel can also be queried to
fetch any previous data since the start of the server including
expired documents/views with timestamps for analysis.
There is lot of stuff that can be added in future. This commit
just lays the foundation of appropriate classes.
Change-Id: Ifcf6c2896ef46b33935802e79cd28240fd4f980e
Reviewed-on: https://gerrit.libreoffice.org/22869
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>
Turns out the callbacks from documentLoad() are in general done on
another thread, not necessarily the same one that did the
documentLoad() call. Not sure why it happened to be the same thread
for me, but oh well. Need to fix the problem described in
3671abf89b in some other way then.
This reverts commit 2fab757462.
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.
Not needed after all. It was a red herring. The device files work fine
even if not owned by root:root and with mode 664. The actual problem
was that I used a file system mounted with nodev when testing loolwsd.
This reverts commit 509314d559
We are not interested in the variable being assigned an incremented
value. Its value is not used any more. We are just interested in the
value of the variable plus one. Using pre-increment gives the wrong
impression.
Sure, this is nit-picking.
Sorry, could not resist. Obviously not very important.
In retrospect, maybe it would have been better to have as policy to
*not* use any 'using Poco::Foo'. Now there is an inconsistent mix of
writing out the complete type and using a 'using'. Plus copy-pasted
long lists of 'usings'. And of course, one should never have 'using'
in an include file. Oh well.
This is possible by moving all the password handling logic to
Document container class. When a user opens a password protected
document the first time, it saves all possible data such as
password, password type etc. Upon opening the same document
again, password is matched with the cached password saved in the
document container class before allowing the new view access to
this document.
Change-Id: Id1f2b6e06de806564bf865e83fed51b01c9a0fbc
Reviewed-on: https://gerrit.libreoffice.org/22208
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Sometimes there are situations when `connection` map is filled
with dead sessions. We don't want to deal with those dead ones.
Change-Id: I00dda77c39b5adbba69421eace0be0159e02505c
Reviewed-on: https://gerrit.libreoffice.org/22207
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
There are 5 LOK callbacks currently that are triggered on LOKit,
and not on LOKitDocument. These include status indicators, and
document password callbacks during document load. Lets move all
the callbacks called during document load in the Document container
itself, and keep the callbacks called after document load in the
Child session.
Change-Id: I8e43c2baaa12023b34822954dd494780ee6dd7ca
Reviewed-on: https://gerrit.libreoffice.org/22206
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Our DocumentCallback is smart enough which checks all the
running connections, and send the callback notification to all of
them. Registering the callback only during the first loadDocument
call should be enough.
Change-Id: I82bcb9525814dae14def3bfb6c088337d0d0ea3c
Reviewed-on: https://gerrit.libreoffice.org/22202
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
