The use of a common threadname suffix in the WSD and Kit
processes is intentional. It is designed to help filter
for a single document's logs across both processes.
The thread name has nothing to do with the classes in
the code, nor is it intended to imply any relationship
except with the process and the document in question.
As the comment in this patch explains, the choice of
the suffix is arbitrary and while it may be changed,
it has to be sensible and common between the two threads
to allow for easy grepping.
Historically, there were in fact dedicated threads
within the respective "broker" classes, but this
fact should be safely ignored, since at the log level
we care less about which part of the code generates a
log entry (that info, if needed, is at the end of each
log entry, in the form of filename and line number),
rather we care more about which document it relates to,
which is crucial in investigating production issues.
Logs and code structure are only incidentally related.
Logs are (or at least should be) designed around
the execution structure, not code architecture.
(This reverts 2a16f34812)
Change-Id: Ic6fe2f9425998824774d2644fe4362e75dea6b88
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101261
Tested-by: Jenkins
Tested-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
We now gracefully fallback to copying when/if systemplate
is readonly.
The bulk of the change is to support proper cleanup in
both cases.
First, we had to move as much of the jail bootstrapping
into the loolwsd-systemplate-setup script, so systemplate
will be as complete as possible before it is locked down.
Next, we needed to update the jail with graceful fallback
to linking/copying upon failure. For that, the jail setup
logic in Kit.cpp has been reworked to support not just
update failures, but also more comprehensive mounting
failures as well.
Finally, jail cleanup now is seamless. To support proper
cleanup when we had mounting enabled but had to fallback,
we mark jails that aren't mounted so we can 'rm -rf' the
contents safely and without fear or causing undue damage
(as unlikely as that is, technically we wouldn't want to
rm systemplate files, if mounting read-only had failed).
There are a few minor refactorings of JailUtil to make
it cleaner and more robust.
Change-Id: Iac34869cb84f45acf64fbbc46d46898367b496d2
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/101260
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Andras Timar <andras.timar@collabora.com>
For various reasons, systemplate may be read-only
or under a different owner and therefore impossible
to update the dynamic files in it.
To support such a scenario, we first link the
eight dynamic files in /etc when creating systemplate.
If this fails, we copy the files.
When creating jails, we always check that all the
dynamic files are up-to-date. If they are, nothing
further is necessary and we bind-mount, if enabled
and possible.
However, if the dynamic files are not up-to-date,
we disable bind-mounting and force linking
the files in the jails. Failing that, we copy them,
which is not ideal, but allows us to ensure the
dynamic files are up-to-date as we copy them too.
Ideally, the dynamic files in question would be
hard-link (or at least soft-linked) in systemplate
at creation. From then on we would bind-mount
the jails and everything would work perfectly and
no files would need updating. This patch is fallback
for when this scheme fails, which should be exceedingly
rare anyway, but which still ensures correct operation.
Change-Id: I09c6f057c49396579aaddb1b8bf4af0930dd4247
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100834
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Tested-by: Jenkins
Reviewed-by: Andras Timar <andras.timar@collabora.com>
Happened for me once in the iOS app, not sure if it was just a random
fluke while debugging, or whether it can happen in normal use. Anyway,
take it into consideration.
If the view count is zero, don't overwrite the '['.
Change-Id: Ia797d7cc99cc273e68ec184406f47283c9a647e2
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/100672
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
The default Poco connection timeout is 60 seconds,
which is probably excessive. The current configurable
default is a more reasonable 30 seconds.
Currently we set this timeout on Storage connections
going out (i.e. WOPI connections).
Change-Id: Ie80a9141ca9bf721addc74baf94e62e0ad72fdd2
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98913
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ash@collabora.com>
Sometimes multiple messages are processed in a single iteration
at socket level. This happens in WebSocketHandler and when draining
Document queue.Just covered these cases.
Change-Id: Ifa46f5d484b67015cca64008b2c89426cc839e64
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/99387
Tested-by: Jenkins
Tested-by: Gabriel Masei <gabriel.masei@1and1.ro>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Gabriel Masei <gabriel.masei@1and1.ro>
The map._activate, among other actions, is sending indirectly some messages
to the server like clientzoom and clientvisiblearea. If these messages are send
before the document finishes processing the load message then there is
a chance that a nodocloaded error will be thrown because there is a
chance that the messages will be processed in parallel with load. This happens
constantly for xlsx files. This is generated by the Unipoll mechanism which,
in case of xlsx files, triggers a parallel processing.
To avoid the above scenario a mechanism of disabling parallel processing of
messages in kit was implemented and is used for load and save messages, for now.
Change-Id: I4c83e72e600f92d0bb4f1f18cebe694e326256d0
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98519
Tested-by: Jenkins
Tested-by: Michael Meeks <michael.meeks@collabora.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Do not send faileddocloading error if it is because of password.
When the document is password protected, we send to the client
both passwordrequired and faileddocloading.
These two are handled differently. While the first prompts the
user for password input, the second internally flags fatal error
and shows an error message that the document may be corrupted etc.
The end result is that the document is not loaded and displayed
when the user submits a correct password. To reset the fatal
error one has to reload, which is unhelpful when we need to
provide a password.
This patch makes sure that we only send one error message to
the client. If a password is required, it already implies that
the document didn't load, and that with the proper password we
should try again.
Similarly for when the password given is wrong. However, if
loading fails and it isn't a password-related failure,
faileddocloading error is returned and in this case the client
handles it as a final error (that requires reloading to retry).
Change-Id: I383418fd40b6e0749b20af0ef8dc40f391a05559
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98676
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
We probably used to have circular references that made KitSocketPoll
and KitWebSocketHandler objects hang around forever, or something.
(Not a problem in web-based Online where kit processes have a
restricted lifetime.)
Change-Id: Ia6eebc51f4a4a8fb4f69a2c83a0131de921ea1d6
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98744
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
This is for the benefit of a next-gen iOS app (without FakeSockets and
much of the current Online plumbing).
This is not supposed to cause any functional changes in normal Online
even if code is organised a bit differently.
Change-Id: Ib09a84ff5d3ba858cf3f50553d76757966af7ad2
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98655
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
"Disable for now - pushed in error" says the comment added in
e11794da25.
Change-Id: Ia2b72bfe20f8ff16d74d1966d511c74eab3e4417
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98587
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
The new utility is safer and more readable.
Change-Id: I3a86675378d458cb004e5534dbf2b401936d0e57
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/98183
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
A small re-factoring to help planned re-plumbing of the iOS app.
Change-Id: I21f09216a7c5adf965179765a75f5a0d521cd7f3
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97771
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
loolmount now works and supports mounting and
unmounting, plus numerous improvements,
refactoring, logging, etc.. When enabled,
binding improves the jail setup time by anywhere
from 2x to orders of magnitude (in docker, f.e.).
A new config entry mount_jail_tree controls
whether mounting is used or the old method of
linking/copying of jail contents. It is set to
true by default and falls back to linking/copying.
A test mount is done when the setting is enabled,
and if mounting fails, it's disabled to avoid noise.
Temporarily disabled for unit-tests until we can
cleanup lingering mounts after Jenkins aborts our
build job. In a future patch we will have mount/jail
cleanup as part of make.
The network/system files in /etc that need frequent
refreshing are now updated in systemplate to make
their most recent version available in the jails.
These files can change during the course of loolwsd
lifetime, and are unlikely to be updated in
systemplate after installation at all. We link to
them in the systemplate/etc directory, and if that
fails, we copy them before forking each kit
instance to have the latest.
This reworks the approach used to bind-mount the
jails and the templates such that the total is
now down to only three mounts: systemplate, lo, tmp.
As now systemplate and lotemplate are shared, they
must be mounted as readonly, this means that user/
must now be moved into tmp/user/ which is writable.
The mount-points must be recursive, because we mount
lo/ within the mount-point of systemplate (which is
the root of the jail). But because we (re)bind
recursively, and because both systemplate and
lotemplate are mounted for each jails, we need to
make them unbindable, so they wouldn't multiply the
mount-points for each jails (an explosive growth!)
Contrarywise, we don't want the mount-points to
be shared, because we don't expect to add/remove
mounts after a jail is created.
The random temp directory is now created and set
correctly, plus many logging and other improvements.
Change-Id: Iae3fda5e876cf47d2cae6669a87b5b826a8748df
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92829
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
- read settings from loolwsd.xml
- in case of notebookbar activated send :notebookbar parameter
- for mobile apps I left empty parameter in setupKitEnvironment calls
Change-Id: I5813589564b37eecc1e77c5d0eb737eca5f92f04
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97233
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Szymon Kłos <szymon.klos@collabora.com>
KitSocketPoll has virtual functions, but a non-virtual destructor. Mark
it as final to make it clear that it's safe to call delete on it.
Change-Id: I685f9d7bcfbb82115e9c25991c877aa99391dd3e
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/97361
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Seems to not cause any serious regressions in the iOS app or in "make
run", but of course I am not able to run a comprehensive check of all
functionality.
Change-Id: I44a0e8d60bdbc0a885db88475961575c5e95ce88
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/93037
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
The tokenizer(s) are more generic than the protocol
logic, and are used from contexts that don't involve
the protocol as such.
Change-Id: Ie8c256bf11a91e466bff794021f41603c9596a7f
More readable and typically more efficient.
Change-Id: I9bd5bfc91f4ac255bb8ae0987708fb8b56b398f8
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/95285
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Tested-by: Jenkins
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
This patch allows the lok core to know about the device form facor of
the client requesting the creation of a new view, immediately instead
of a later time.
When a request for a new view is sent a 'deviceFormFactor' parameter
is appended to the message.
This parameter can have one of the following values: 'desktop',
'tablet','mobile' and is forwarded to the lok core.
Change-Id: I21739ddb8c43c960164b3c625e4cf0a80f4616a4
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92691
Tested-by: Marco Cecchetti <marco.cecchetti@collabora.com>
Reviewed-by: Marco Cecchetti <marco.cecchetti@collabora.com>
is a follow up of 217dd2de54
"Do not broadcast view-cursor invalidation messages"
But this change is implicitly Calc specific because
LOK_CALLBACK_CELL_VIEW_CURSOR is used only in Calc.
This patch fixes the following bug:
Suppose there are at least three clients each with a different zoom and the
all client's cursors are placed in same tab-view and away from A1. Now
it can be seen at once that the cursors of other clients in each client
are rendered incorrectly.
The commit c6b18508aec0e609b4c3c000faf89c21e93620bd in core.git
"lok: send other views our cursor position in their view co-ordinates"
does the right thing by sending view specific cell-cursor positions.
But the broadcast of these messages in Kit.cpp means every client will
get the messages intended for others and possibly end up using the
wrong messages to draw the cell-cursors.
Change-Id: I6e9534c2e562f34b5c1fe37242b36e076b9319c8
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92916
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Dennis Francis <dennis.francis@collabora.com>
Sometimes kit process goes into a heavy processing state (or even hangs)
and is not able to report its memory usage. Thus we can't implement cleanup
of problematic kit processes based on memory information reported by kit.
By moving memory reporting to admin module we avoid this problem.
Change-Id: Icf274e3a3a97b33623a93f9d2dc1e640ad9b7d99
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92752
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
This is needed in case the original template
directory itself is linked, which after setting
the FTW_PHYS flag to nftw skips them.
Change-Id: I63141b64ca486e6e2e979cdf1d80fe0fd0f3990c
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92104
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
And some formatting, but no functional changes.
Reduced the maximum time before logging all
link/copy activity of jail files, which typically
takes < 200ms.
Change-Id: Ie48072314471195a5f156cb45c616a5e57d2a287
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92103
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Poco's File::linkTo was added in version 1.8.1 and
we still support older versions. Also, symlink(2) is
far more transparent and simpler here.
Change-Id: If537cc77cd1388f3c0e2a6b16b1edcf46a60e357
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92102
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
in kit as these are sent by core to specific view(s) by ultimately calling either
(as seen from the usages of LOK_CALLBACK_INVALIDATE_VIEW_CURSOR in core)
1. SfxLokHelper::notifyOtherView() where it sends to particular view.
or
2. SfxLokHelper::notifyOtherViews() where it sends to all views except
the current view.
The core makes the decision to broadcast or not, and if it does, then
the kit's broadcast of broadcasted messages can only cause a blowup of messaging
complexiity w.r.t number of views. It also does not make sense to send
view-cursor messages meant for a view to be sent to others in Calc
with clients of hetrogeneous zooms.
Change-Id: Ib07c5fbba9bb05c59048561d2c26aed00f3be598
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92633
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
It was made static in ea2b77ce07 for
Android's sake, so returning it to be per-instance is not a big thing.
Keep a separate static just for the Android app's use for now, while
the Android app supports just one open document at a time anyway. (It
is for the iOS app that I am moving towards supporting multiple open
documents at a time.)
Change-Id: I7fabeb21883eb7cd7155e880eb4cc0413124d1f8
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92625
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
It is not a problem in the multi-process web-based Online, where the
variable exists separately in each KIT process (which handles exactly
one document). But in a mobile app, when we want to be able to handle
multiple document in the single process, we can't have such variables.
Change-Id: I1d3da48316eb3a8c72ff4957cc3fcba8f6870f16
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92582
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
It just causes confusion with the name "DocumentBroker".
DocumentBroker objects exist only in the WSD process. Instead just use
"kit".
Change-Id: I3d9915c4759899ea6ed9084cf3ec6dc0f3b88ee5
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92474
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Tor Lillqvist <tml@collabora.com>
gdb will take it from the system anyway, so it's just amazingly
excessive weight wrapped in a performance problem.
Change-Id: Ie8d7d2be97da64233a6d7cb4864d6ee88ea8c337
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92207
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
The switch away from LOOLWebSocket and the use of a websocket
for talking to forkit removes the need for the pipe code.
Change-Id: Ifb0c6c88681289e7a1709d9bc3281532935c7be4
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92033
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
This mends several problems from commit
5710c86323.
Change-Id: I1b29f29ca81679608a2692488fa1ef22b2e62dfd
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/92032
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
In linkOrCopy, the nftw() function is used without the FTW_PHYS flag to
populate child roots from systemplate. From man nftw:
FTW_PHYS
If set, do not follow symbolic links. (This is what you want.)
If not set, symbolic links are followed, but no file is reported twice.
Because the order in which directory entries are visited is not defined,
having multiple symlinks to a file results in only one of the paths
being created in the chroot.
This is not really a problem because loolwsd-systemplate-setup creates
systemplate without symlinks. Fixing it might prevent unpleasant
surprises in the future though, and might possibly allow to make
systemplate and chroots smaller (also the manpage says that you want
it:)).
The commit adds FTW_PHYS flag to the call as well as symlink handling.
Change-Id: I01354f529b5d340185988ed026f266caf17a6881
Reviewed-on: https://gerrit.libreoffice.org/c/online/+/87749
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Michael Meeks <michael.meeks@collabora.com>
Andras Timar
Miklos Vajna
Ashod Nakashian
Tor Lillqvist
Gabriel Masei
Michael Meeks
Pranam Lashkari
Szymon Kłos
Marco Cecchetti
Dennis Francis
Martin Milata