In the rare event that it fails, we will
cleanup the DocBrokers correctly.
Change-Id: I6f5346c9ec5021bdadc8a01ed389951ca18d07a3
Reviewed-on: https://gerrit.libreoffice.org/32676
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
When saving we need to differentiate between no-op
and failure. The lastSaveTime must always be updated
when saving didn't fail (i.e. no modification or saved).
Change-Id: I0e2455afac22c82f0b623f9441fbc0bca8a7cb83
Reviewed-on: https://gerrit.libreoffice.org/32629
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
While time_t is much simpler, it's too
opaque. The new chrono library is type-safe
and does conversion correctly, as well as
guarantees monotonity and other desirable
properties.
Change-Id: Id41c44c397a31d73e894e8f1715ff18f2b67df53
Reviewed-on: https://gerrit.libreoffice.org/32627
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Since we already enumerate the DocumentBrokers and
remove dead ones before every autosave (every 30 seconds)
as well as when loading/unloading documents, there
is little reason to have a separate timeout and loop
for idle documents.
In addition, the previous code only killed the child
but didn't remove the dead DocumentBroker entry from
DocBrokers. Not clear if this was intentional, but it
would have been removed on the next cleanup anyway.
Change-Id: Ie1e09c77133165dd1255930b0b0be06fde7646b1
Reviewed-on: https://gerrit.libreoffice.org/32626
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Between waits on forkit we shouldn't spend too much time.
This is to recover from forkit crashes.
Now we first spawn children, and only when not successful
(i.e. no more spare children needed) we check for autosave
and DocBrokers cleanup. Only when none of the above is done
do we sleep.
This gives the best balance between forkit waits and reduces
the unittests by a good 25 seconds (crash tests down from 34s
to about 10s only).
Change-Id: If69284746859bc78d14f1c6eda07aef4e006709e
Reviewed-on: https://gerrit.libreoffice.org/32624
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Otherwise we throttle spawning to allow
time to fork the process. If we don't wait
we can bomb a loaded server and bring it down.
However during startup this is not necessary
as there are no in-flight spawn requests.
Change-Id: I1beac94571f6d8d96136d32c81310bea6547242b
Reviewed-on: https://gerrit.libreoffice.org/32622
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And say 'forkit' in the logs where we recognize it
instead of the generic 'child'.
Change-Id: I7628b064bb6330db145a948640e48b727def3270
Reviewed-on: https://gerrit.libreoffice.org/32619
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
And check for save-failed first before finding
the session, as otherwise the session is useless.
Change-Id: I3eb45e54872547eb36765b2c8409e1aa51aee589
Reviewed-on: https://gerrit.libreoffice.org/32611
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Avoid unnecessary code under lock
or when DocBrokers is updated,
which requires removal on exception.
Change-Id: Id01aed42cd66616b910b7e16a8a1ed6c1d1e74b9
Reviewed-on: https://gerrit.libreoffice.org/32558
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
... and print error in the logs. So that if there is some problem
in the setup and we are not able to interact with WOPI host for some
reason, we print it in the error logs clearly.
Convert to new style logging - LOG_XXX macros.
Change-Id: I53ce4f61136ffd256b0eae8712dc7f22a620e2bf
Simplifies the code and makes it less error prone.
Also add more logging on modifying DocBrokers.
Change-Id: I861495912eb4994a32b1ccaccc181fb1aad26e1f
Reviewed-on: https://gerrit.libreoffice.org/32295
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The WS could be chocked on write, but we shouldn't
assume the child is dead because of that. We are
only trying to test if the child process is alive
in that helper.
Change-Id: I108a297e43f923cab0dfa30204837dc4df15d3a1
Reviewed-on: https://gerrit.libreoffice.org/32289
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Since the local path already ends in foreslash,
no need to keep the on in provided by the client.
Change-Id: Ia2bc24c7faa84509f9ec18deefb14cad2858e856
Reviewed-on: https://gerrit.libreoffice.org/32288
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
For now, do the check only when a new session connects to the
document, because at that point we fetch the document information (in
separate function for WOPI and local files) and construct the
FileInfo, including timestamp.
For now, just log an ERR message if we notice that the document in its
storage system (WOPI or local file system) has an unexpected last
modified time. What should we do? If we don't have unsaved changes,
most likely we should just silently reload the document and force all
sessions to refresh. But if we have unsaved changes, and the document
has changed underneath, we have a problem.
We need to fetch the timestamp also also after saving ("persisting")
as we can't assume that the clock on the machine running loolwsd and
that of the storage (as reported in the WOPI case in CheckFileInfo)
are in synch. (Assuming separate machines, they certainly won't ever
exactly in synch, but aren't necessarily even just a few seconds apart
(think incorrectly set up timezone etc), so no amount of tolerance in
the comparison would be good enough, because after all, it might be
that in the problematic cases we are looking for the timestamps also
are separated by a quite short time.)
Yes, this means there is a race condition; what if the document is
modified behind out back right after we have persisted it, before we
ask for its timestamp? It would be much better if the persisting
operation atomically also told what the timestamp of the document in
the storage is after persisting, but alas, WOPI doesn't do that.
Rename the DocumentBroker::origDocumentLastModifiedTime field to
_documentLastModifiedTime as that is less misleading. It is not the
"original" document timestamp but the timestamp of the document in its
storage system.
This needs much more work: Ideally the timestamp of the document in
its storage system should be retrieved and checked against the
expected value also before we are about to save it.
But unfortunately experience has shown that the WOPI CheckFileInfo
operation can be expensive, so we'll see what can be done. Ideally
WOPI should contain the optional functionality to return an error if,
when saving a document, its timestamp (and size?) in storage are not
what the saving client expects.
Also add a few FIXME comments.
Change-Id: I5a9b55d4b55a8db0c9ee8638edd368dc0aa325d5
The information in StorageBase::_fileInfo is not expensive to
construct. What might be expensive we were doing anyway for each time
a session connects to the document. So just update _fileInfo whenever
we fetch currrent information about the document in its storage, which
is useful, as the timestamp and size after all will change whenever
the document is persisted.
Change-Id: I173394c88b4d6448ad5bf1ab9b41694cffdf1ff4
And not as the current time. The getLocalFileInfo() and
getWOPIFileInfo() functions will reset it anyway.
Change-Id: Ief821482cf789113cecd85e5d59afecf6ad2c3ab
It would be another thing if it would be abstract and overridden in
the concrete derived classes. But now it just returns the _fileInfo
field that is set by the separate getLocalFileInfo() and
getWOPIFileInfo() functions. Possibly this should be re-designed
and/or stuff re-named for clarity.
Change-Id: I30d01277ac0c6bd4b9daa317aade319b6ef39342
Poco::Timestamp::fromEpochTime() is a static member function that
returns a Timestamp.
Let's hope this doesn't introduce any regression.
Change-Id: I0997c4c3128ec07d5db76dbf3ecc388cd19ac2ac
Initial effort was setting it when we are compiling with
--enable-debug or when not, unit tests are running, but since we don't
run the unit tests in absence of --enable-debug anyways,
there is no use for this #else code block.
Change-Id: I9d7c66a18be160a47afd2bf336d89d50d4f2463e
loleaflet_logging defaults to true with compiled with
--enable-debug otherwise false.
Browser will print additional debug info when this property is
set to true.
Change-Id: Id9fabf134bd8d19fa1a09ca8c0987df46d4f1a4c
By that time the loleaflet code has already done the greying-out (this
happens at the latest after 10 minutes of inactivity) and marked them
as 'inactive'. Bluntly closing the session seems to work out fine, no
new horribly alarming message is displayed by the loleaflet code in
addition to the already visible 'Inactive document ...' one.
Change-Id: I420f0d7530194e6c8d6f34d7985ab810cde5a76a
To avoid degrading performance for everyone
because of a single slow/bad connection, we
send data to clients each in its own thread.
Change-Id: I6f980c25a404c4d05bcdb1979849ea3d2776c7b9
Reviewed-on: https://gerrit.libreoffice.org/31984
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
The call to the setRaw() virtual function during construction will not
dispatch to derived class, so the ctor is correct only in case there are
no subclasses.
Change-Id: I484d276dd05e53211b513373100b70586a5857c0
using these WOPI protocol extensions: DisablePrint,
DisableExport, DisableCopy. All assumed to be false if not
provided.
Change-Id: I415597d710107f9d8cbb8757f361365cc2a88eb1
We want document owners (sessions with their WOPI UserId =
OwnerId) close the document (for all sessions) only if
EnableOwnerTermination property is specified. Note that
EnableOwnerTermination is an extension to WOPI, and not part of
original WOPI standard.
Change-Id: I7237d172c4c54477572bc132336910250af075a3
The pool is checked for expansion before processing
items from the queue. By tracking the number of idle
thread (i.e. not currently sending data) we ensure
that there is at least one idle thread before
we invoke the socket.
If there is not enough idle threads at that point,
a new thread is spawned, so long as we're below the
limit. This guarantees that even if all the active
threads block on the socket, we'd always have one
more to process new data (until we reach the limit,
which is as many client connections as we have).
Technically, a single slow connection could
still monopolize all connections if there are
many messages to be sent to it. For that we'd
need to track and assign one thread per connection,
something we don't currently do.
Change-Id: Ic8b5e064da068b37bcfa773005495b198763c31d
Reviewed-on: https://gerrit.libreoffice.org/31886
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
This adds SenderQueue and a wrapper of messages to
send back to clients.
Currently no threading takes place, but the messages
are pumped through the queue nonetheless.
Change-Id: Id9997539c0a2a351cbf406f649c268dd3643e88e
Reviewed-on: https://gerrit.libreoffice.org/31883
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Use a new protocol message, 'resetidle' to inform Admin clients
whenever a user has done anything in a document view. This is a
message that Admin clients need to subscribe to.
Also add the current idle time for each document to the 'documents'
message.
To reduce protocol chatter, the idle time is updated at most once per
10 s.
Change-Id: I418e82b05048a3628f21dcd240ccd974b3a01356
Reviewed-on: https://gerrit.libreoffice.org/31653
Reviewed-by: Tor Lillqvist <tml@collabora.com>
Tested-by: Tor Lillqvist <tml@collabora.com>
Set a timer in loleaflet 15 minutes before access token expiry
date (access_token_ttl value) to prompt the user to save and
refresh the session.
Change-Id: I98c3e47c9b7031e29e002f653d488747b9c17df8
Reviewed-on: https://gerrit.libreoffice.org/31381
Reviewed-by: Jan Holesovsky <kendy@collabora.com>
Tested-by: Jan Holesovsky <kendy@collabora.com>
We now guarantee that forkChildren will be invoked
under DocBrokersMutex lock.
This eliminates the case when alertAllUsersInternal
is invoked when this mutex isn't locked.
Change-Id: Ibb259bbb4f380300a90ad2fc7affe6013dd71fef
Unfortunately this brings back the deadlock that
51c88c5fb7 fixed :-(
This reverts commit da3b1b208a.
Change-Id: If48c0b3ddebf3fb366786d90cb35c3c22963b1a1
Fixup the part for EMPTY invalidation as well.
N.B. Should replace 'EMPTY' with '0,0,INT_MAX,INT_MAX'
to be consistent. Since both are expected, no compat
issues should be expected.
Change-Id: I066981622b1de71e9e849530df0583291b74b804
Reviewed-on: https://gerrit.libreoffice.org/31393
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
Messages larger than a certain size are preambled
with a 'nextmessage' message that hold the size
of the subsequent message.
This is a workaround to a limitation the Poco
WebSocket API where if the buffer size is
smaller than the received frame the socket
ends up in a bad state and must be closed.
Unfortunately the new API that avoids this
workaround is not yet released by Poco.
Here we minimize the need for 'nextmessage'
to truely large messages. The limit is now
raised from above 1KB to over 63KB.
We may raise this limit further, but that will
cost each socket that much dedicated buffer size.
Change-Id: I01e4c68cdbe67e413c04a9725152224a87ab8267
Reviewed-on: https://gerrit.libreoffice.org/31286
Reviewed-by: Ashod Nakashian <ashnakash@gmail.com>
Tested-by: Ashod Nakashian <ashnakash@gmail.com>
