That global flag is checked all over the place, so setting it will
actually make the threads eventually finish. (All polling is done with
timeout, I think, and then checking TerminationFlag whenever the poll
times out.)
Sure, it would be much better to use an eventfd and poll that, too,
instead of timing out from the polls all the time to check a plain old
boolean flag.
If you can't come up with a meaningful message to use in a
CPPUNIT_ASSERT_MESSAGE(), just use CPPUNIT_ASSERT() instead. These
messages aren't intended for end-users but for developers, so it is
pointless to make them high-level and dumbed-down.
In case of abnormal termination of session from client-side,
we might still have data being processed in the kit process, for
example, during auto-save. Trying to send such data over an
expired socket towards the client would throw exceptions which
need to be handled, otherwise the auto-save process would not be
successful. Further, the unhandled exception can bring the document
broker in an unstable state with dockey still present in the
object.
Also do not send 'editlock: 0' to a websocket session which is
going to be removed because the session might have been
forcefully terminated from the client-side, in which case sending
data would go nowhere.
Change-Id: I10eb9c818bc81d4db26d5a19dc8bd44f6fbdf32c
Enforce user being 'lool' for setcap binaries loolmount and loolforkit.
Add warnings if configured without --enable-debug.
Developers should pass --enable-debug to configure.
The loolwsd process created it and opened it for reading, but nothing
opened it for writing.
There is still documentation for it in README, that needs to be either
rewritten to match reality or removed.
Comes in handly in some testing situations where you don't want to
send a signal to get loolwsd to finish. Option is present only in an
--enable-debug build.
If the first session used to save does not hold the edit-lock,
all the .uno:Save commands will get converted to dummy messages
and never reach the kit process.
Change-Id: I36cbee778f8c2c5866dcf276cf156fdf9ed8388e
It is a status code, a 2-byte unsigned integer in network byte order,
potentially followed by a textual reason. But we never include any
specific status codes in the CLOSE frames anyway. (With a Poco-based
WebSocket peer it is always WS_NORMAL_CLOSE, 1000, 0x03 0xE8.)
Since WSD now takes into account the fact that
ForKit always spawns one child, the unit-test
shouldn't expect +1 preforked children.
Change-Id: I5cbe9d817a0d2ffdf9fb0953ef85450f7b8b224f
Reviewed-on: https://gerrit.libreoffice.org/23980
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
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 <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Connection thread should not attempt to disconnect the session,
which in turn will try to unload the document, which will
wait on the connection to destroy. The latter will never
happen since the connection destructor must, correctly,
wait for its thread to finish, which is waiting on itself now.
Since the session disconnect is already called from the session
destructor, there is no need to explicitly invoke it here.
Change-Id: Iaf9e8a10d4caa9001208084e909a14b4d4c5105e
Reviewed-on: https://gerrit.libreoffice.org/23966
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
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 <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
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 <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This seems to get rid of the "terminate called after throwing an
instance of 'Poco::SystemException'" problem for me at least.
Sigh. Why can't the compiler warn about such things? I build with
clang++ -Wall -Wextra. The Connection class is fully defined inside
the LOOLKit.cpp so it should be able to, right?
Presumably it is only developers that are interested in signals, and
terms like SEGV or ABRT are more precise than their textual
descriptions like "Segmentation violation" or "Aborted".
Sleep a second before exiting in case we get a fatal signal just when
about to finish, which sadly seems to happen often. (In fact, if
handleFatalSignal() is running at the same time, it will kill the
process so we never get to the _Exit() call.)
Also, display more information about the exception.
Actually I think I should factor out the code to display as much
information as possible from an exception. Currently the amount of
information displayed varies from case to case in the code-base.
Poco::Channel is reference counted, but the initial refcount is 1, so we
need to release channel in order to have it deleted when Poco::Logger
releases it.
Calls to Poco::Logger::shutdown() are still missing though (from
forkit/kit/wsd).
Change-Id: I12ab32047d32e55902c60639d71eb6ef30ffa3bd
Put this inside #if, add the 'override' back for current Poco. Avoids
"warning: 'expectContinue' overrides a member function but is not
marked 'override'" when compiling against current Poco.
Not sure what good it would do to define it at all if compiling
against a bleeding-edge Poco as it won't call it anyway?
The logging functions already display the thread name on all output
lines. No need to mention it another time in the thread start and
finish logging messages.
A call to Log::error() should be enough to indicate that it is an
error. We don't need to prefix the message with the string "Error: "
in some cases but not others. (If we do want such a prefix for all
errors, surely then we should add it in the actual Log::error()
function.)
Also, change some more Log::error() calls to Log::syserror() where
appropriate.
Much better than assuming that errno would be relevant at all
Log::error() calls (or alternatively, having to remember to append a
false parameter to the Log::error() call, which had not been done a
single time anyway.)
Call log::syserror() right after a system call has returned an
error. Don't call it otherwise.
Loading documents from the local filesystem
opens the door to security issues.
By default filesystem storage is disabled,
even if enabled in the config file. The
only way to enable it is to set the
allowlocalstorage command-line argument.
Change-Id: Ib8f57377260817436d101a16757aab38276cbdcd
Reviewed-on: https://gerrit.libreoffice.org/23881
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Most all configuration values can now be defined
in the configuration XML. The command-line arguments
can be used to override some of these values (for
the convenience of developement and testing) and,
in a few cases, as a security measure to avoid
storing sensitive data in the configuration file.
Change-Id: I040b807b1a59a3537bb94646150d3c7d711f8b62
Reviewed-on: https://gerrit.libreoffice.org/23880
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
A new command-line argument, admincreds, must be provided
to set the Admin Console credentials.
The new command-line argument specifies the username
and password in the following format: username/password
If not provided, Admin Console is disabled for security
reasons. A warning is emitted at startup and an error
on every invocation of Admin Console is logged when
no credentials are defined.
Change-Id: I348623949fd0b292f5066e4955759c708204540f
Reviewed-on: https://gerrit.libreoffice.org/23878
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
loolstat now uses `pgrep loolwsd$` instead
of relying on pid dumping in a temp file.
With the Admin Console this tool (loolstat)
is less useful, so this cleanup is probably
a stepping stone to removing it altogether.
Change-Id: Ib7732a00c3d3ea54dffcb71e9fe1a56c4a88016e
Reviewed-on: https://gerrit.libreoffice.org/23877
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Admin no longer needs a pipe as it's notified
from WSD. It is now a singleton with improved
locking.
The tracking of documents and views still needs
improvement and corrections.
Change-Id: If614331de6dd595c6dd4443f480d4ab588ca4551
Reviewed-on: https://gerrit.libreoffice.org/23860
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
To use such enums would be a mistake. It is quite enough to just use
the message tokens as strings. Duplicating them as enums will just
lead to the enums getting out of synch (as they already were). We
would also need functions to covert between the string and enum
forms. It seems to be hard enough to keep the messages documented in
protocol.txt.
Add a function to determine whether a client message indicates user
interaction. We need that distinction when deciding when to do an
automatic ("idle" or "auto") save of document being edited.
"Interaction" is a loose term, possibly what we actually want is to
see whether the user is actively doing an edit that changes the
contents of meta-data of the document.
In the instdir/program of an --enable-mergelibs build of LibreOffice
there is also a dummy libsofficeapp.so. Obviously we don't want to
even try that. So look for libmergedlo.so first.
I easily got the thing into a state where it kept forking new
processes without limit even if no client was doing anything. The log
output contained lines like:
wsd-09526-04 00:00:48.302029 [client_ws_001b ] MasterToBroker: spawn -1
brk-09528-00 00:00:48.279945 [loolbroker ] Broker command: [spawn -1].
brk-09528-00 00:00:48.279964 [loolbroker ] Spawning -1 childs per request.
brk-09528-00 00:00:48.279974 [loolbroker ] Creating -1 new child.
This fixes that, but there are still several issues remaining.
So just use DEFAULT_CLIENT_PORT_NUMBER and don't confusingly include
ChildProcessSession.hpp in the test program sources just to define a
static (file-local) ClientPortNumber variable that never gets set to
anything except its initialized value. ChildProcessSession is specific
to the internals of loolwsd and loolbroker and should not be used in
the test programs.
Preiniting LibreOfficeKit and forking kit processes (instead of
spawning) has worked fine for a while, and has been the default way
this works.
No 'loolkit' program gets built any more.
Convert-to is broken after re-designing Broker
and simplifying it. Temporarily disabling it
to help `make check` pass until it gets fixed.
Change-Id: Id49f86d8a1a25367233a09b865630ed3a210a4fd
Reviewed-on: https://gerrit.libreoffice.org/23793
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Broker is now just a simple spawn-machine.
It only gets requests from WSD to spawn
new kit instances, which it doesn't even
track.
Once a kit instance is initialized, it
connects to WSD with a control WS.
From there on, it's up to WSD to manage
the kit process.
Also was removed the benchmark logic
since it can no longer function.
Change-Id: I1bf56bc6416c9eadafba637276bbb8b3107e5727
Reviewed-on: https://gerrit.libreoffice.org/23790
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
WSD now communicates on a WebSocket directly
with kit processes. ChildProcess encapsulates
kit processes and the control WS, which itself
is owned by DocumentBroker.
Change-Id: Ica209aaa07974739b8e51a14e11325d084e193f6
Reviewed-on: https://gerrit.libreoffice.org/23789
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
In face of exceptions, the lock was not released
and the condition variable was not signalled,
thereby causing all subsequent views on the
same document to fail loading.
Change-Id: I18d3cefcc74a158facefe1e74a9c802ee048b014
Reviewed-on: https://gerrit.libreoffice.org/23785
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
of the 10k files still linked into the jail; 5700 are from usr/
so bind mount just that directory, also set noatime, ro, and
some other helpful looking options.
Change-Id: I28d2d5cbbdf33fb57ea0f0c0915cb267603ee16d
Reviewed-on: https://gerrit.libreoffice.org/23777
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Create directories top-down and not bottom up for more efficiency too.
Skip the sdk (if we have it) - ~20k files, and misc. other
pieces that we don't need; still more to go there.
Change-Id: Iccd9ebac495fba414d268b339ef82a161d98a9ca
Reviewed-on: https://gerrit.libreoffice.org/23770
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This reverts commit 97c8f35ddf.
Since the Broker design has been extremely simplified,
all communication between Broker <-> Kit are gone.
Only a pipe between WSD and Broker remain.
Temporarily reverting this to apply the Broker redesign,
after which this patch can be reviewed and merged.
This will be easier than trying to merge the redesigned
Broker on top of this.
Change-Id: Ia901fad604008654c01841df62e88918adad45e1
Reviewed-on: https://gerrit.libreoffice.org/23769
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
There is nothing that says a client has even requested any tiles, so
there might be none to persist. Don't let an exception thrown by the
DirectoryIterator propagate upwards and cause potential
issues.
Noticed the issue when testing using the 'connect' test program,
giving it input that did not request any tiles.
Had to add a shared pointer to the BasicTileQueue for the session to
the MasterProcessSession object, and restructure the coe a a bit to
allocate BasicTileQueue objects dynamically. Possibly just passing a
reference to a BasicTileQueue in the stack would have worked, but why
risk it?
The actual logic when to do auto / idle save is not quite right still,
did not change that in this commit.
Time increment is handled on client-side, server only sends the
elapsed time during first page-load.
Change-Id: I73e98fd95ca9f391b625a8dcfc7e3490878c6a40
... which can be taken only one at a time. Others can only view,
not edit. When a session with edit lock exits, the edit lock is
handed over to the next alive session.
Change-Id: I712a4e70369f1d07c1d83af416a0f5c288b05c7d
Don't embed newlines in "lines" written to the log. When logging stuff
read from or written to the fifos, translate newlines to " / " for
clarity.
(If we would want complete, exact verbose logging, we should be really
pedantic and log all non-printable bytes in hex anyway, etc, so
displaying newlines as space-separated slashes should be OK. It isn't
as if there would be totally arbitary data passed through the fifos
anyway.)
We will switch to reading all these values as soon as wip
configuration file object is accessible globally.
Change-Id: I16eff339349683050be1985eefabc34854cccba3
We act as a client when we communicate with owncloud server.
For time being, just accept whatever certificates owncloud server
gives us. We might want to get more strict here in future.
Change-Id: I4813d19412b66ecf57d6cdef9c3ac94fbbaa521f
Spare child processes are now in a separate
container. A FIFO that gives older instances
priority to avoid using instances too young
to have initialized fully.
In addition, spare instances are now
proactively spawned such that there
is at least a minimum number of
spares at any given time.
Change-Id: Ibdb206d88473adb306c274f4af39798c784258a0
Reviewed-on: https://gerrit.libreoffice.org/23647
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
