Commit graph

1047 commits

Author SHA1 Message Date
Michael Meeks
3feb7fe58c bgsave: don't process left-over tile messages from the KitQueue.
If we had a tilecombine processed in the same queue and before
a save - the 'render tiles at the end' state would be inherited by
the background save process and result in excess work, and also
binary messages where only text messages are expected returning.

The rest of the queue is cleared post fork.

Add unit test - plus hook to hold queue processing.

Change-Id: Iee937897000bc3ac149599844f1eab005affb131
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-05-10 16:30:37 +01:00
Michael Meeks
8b7549b8f7 bgsave: lower priority of background thread.
Free CPU cycles for interactive processes.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I9d8cfe5f3541c9424d51c69380e1c72920154ff4
2024-05-10 16:30:37 +01:00
Caolán McNamara
34b75306c3 AddressSanitizer: memcmp issue
/usr/bin/coolforkit
	memcmp
		asan/../sanitizer_common/sanitizer_common_interceptors.inc:875 (discriminator 9)
/usr/bin/coolforkit
	(anonymous namespace)::textItem(std::vector<char, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool&)
		online/kit/KitQueue.cpp:52
/usr/bin/coolforkit
	KitQueue::put(std::vector<char, std::allocator<char> > const&)
		online/kit/KitQueue.cpp:94

since:

commit f8a0d6c086
Date:   Thu May 9 15:29:49 2024 +0100

    Callbacks: minor efficiency wins, avoid tokenizing where we can.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ie5d0f86bb33c810b6a5f177421485c977b7fc79a
2024-05-10 16:16:49 +02:00
Michael Meeks
d4d4731136 Cleanup condition check for a quick poll.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ica75605d3a12e7eae86cc0e4cbebb722aef92919
2024-05-09 17:47:15 +01:00
Michael Meeks
f8a0d6c086 Callbacks: minor efficiency wins, avoid tokenizing where we can.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I2ef647fd0af7bc83b7b40fd3ce7a49dc799f4339
2024-05-09 17:47:15 +01:00
Michael Meeks
6f49f9398e Split outbound callback processing from incoming message queueing.
Now we always send callbacks as soon as possible back to wsd from the
kit. This has several implications:

1. even when InputProcessing is disabled we will send outbound
   progress updates.
2. we should send callbacks much more quickly without waiting for
   other queue events to be processed eg. tilecombine:

We also drastically simplify storage of callbacks, avoiding lots of
re-parsing of the same strings, and allow much more efficient
comparison and merging at a small space cost in queue size.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ia1ede5406767f895616a52775316ee6ab1c5db09
2024-05-09 17:47:15 +01:00
Michael Meeks
320606c225 KitQueue - remove more un-necessary code, and helper use.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ic11db8ac1391bd22b4adcae40c99fa4ca99790f1
2024-05-09 17:47:15 +01:00
Michael Meeks
f4265ce8a7 KitQueue - move methods from header to source and simplify.
also rename and dissolve the previously virtual _impl methods.

Change-Id: Id77a5c3ce79b27406301ef336ac1c9a40ead15bf
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-05-09 09:50:08 +01:00
Michael Meeks
ebb4eda3df Move common/MessageQueue.cpp -> kit/KitQueue.cpp.
This is not common code, it's used only in the Kit.

Rename TileQueue -> KitQueue as well as unhelpful member variable naming.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: If158597f99f302cbc27e8eab139972f57a2fc3bb
2024-05-09 09:50:08 +01:00
Michael Meeks
466c31d59a MessageQueue cleanup, and avoid accidental use in tests.
The MessageQueue is not a generic message queue, it has much more
un-anticipated functionality; so don't use it where we don't need to.

In particular unexpected re-writing and merging of messages
during tests is probably not a great idea.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I657738307e611be18f5f83e11c055bf8a88826da
2024-05-09 09:50:08 +01:00
Michael Meeks
5357b0e81f bgsave: update unit tests to exercise races in modified state.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I60d861f78b592fc39b88c23c0dece771ec8260a3
2024-05-08 16:42:29 +01:00
Michael Meeks
c66940cf9a bgsave: close race of typing while a background save completes.
We need to mark core unmodified so we can track modifications
to the core that date from after the background save process is
forked.

We avoid telling WSD about our new modification status until
we are sure the background save completed successfully, and only
in the case that we have not been subsequently modified.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I2c9fbce942ff0af2bb727c3685f4ec479e18fa27
2024-05-08 16:42:29 +01:00
Caolán McNamara
cf01a0687b cid#365236 Dereference null return value
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I325404441747611985266bf51c53bf14423491fc
2024-05-07 15:59:55 +02:00
Michael Meeks
72bbc375be Add more comment warnings around security critical code.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I1ca4a54d076a8f95850f329168c941806bc56b16
2024-05-07 13:11:30 +01:00
Michael Meeks
90f387cdc5 bgsave: change statusindicator protocol message to 'progress' + JSON.
We need to get more helpful structure into this message, and add a
type=bg flag to handle background progress messages.

Add unit test for merging progress: setvalue commands.

Simplify ProtocolHandler sendTextMessage with a std::string wrapper.

Android code needs manual testing.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I54ce807e2fc5de80118905e68557a95e637fbd18
2024-05-06 20:28:08 +01:00
Caolán McNamara
6d00ed64d6 _sessionUserInfo tracks disconnected sessions
as well as current ones.

i.e.

/// User Info container used to store user information
/// till the end of process lifecycle - including
/// after any child session goes away

so add some info in the log to flag which ones are still connected and
which ones are not.

https: //github.com/CollaboraOnline/online/issues/8943
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I5350c04d1a7bb8095464881fba97e5910f71ffb3
2024-05-01 17:51:02 +02:00
Caolán McNamara
c2f4f8031d disable watchdog while loading and saving
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ic1757f1cafcaed7feb3ce0cbc21fe8e03c5d4bd4
2024-04-30 17:55:13 +02:00
Michael Meeks
31624e869c test: cleanup logging accounting post fork, for more helpful tests.
coolforkit: common/Log.cpp:677: void Log::shutdown(): Assertion
`ThreadLocalBufferCount <= 1 && "Unstopped threads may have unflushed
buffered log entries"' failed.

Was caused by mis-accounting. Also join document threads on 'exit'
which happens during unit tests to ensure we don't hit the above.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I523e723e54e4252ae0d65de36af086e97dd79f04
2024-04-29 10:56:54 +02:00
Michael Meeks
12310c7dec test: allow an exitTest to work in Kit and propagate its result.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I0d32d46e81eb3ed42d8531860ef2d8e06bdca591
2024-04-29 10:56:54 +02:00
Michael Meeks
642da39dc5 test: create UnitSyntheticLok - to stub and override LOK behavior.
Simple example to re-instate previous unit test.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I26da1178bc897797a656eb5ae9f838d17cbaf75f
2024-04-29 10:56:54 +02:00
Michael Meeks
14e3a20e18 DocumentManagerInterface - remove un-helpful abstract base.
This added 200+ lines of code, made things harder to
understand, and was used only in a single test that can be
covered in another way.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I4ed8d1d52d533f8b24be7dd5e12dbb7702ef1a80
2024-04-29 10:56:54 +02:00
Michael Meeks
08e17a0388 Simplify Document creation, and coupling to KitWebSocket.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Iec7dfd3d33de2ae548ca73081d50361958672e4a
2024-04-29 10:56:54 +02:00
Michael Meeks
1b6af5e28d killpoco: remove lots of redundant JSON includes.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I976c5b8d6763cbbf0ee5cadfa2f7335ec719fe85
2024-04-25 09:06:13 +02:00
Michael Meeks
884a841cde Logging: wrap poco and simplify logging.
Centralize more logging functionality in Log.cpp, simplify and
wrap underlying logging APIs better.

Code is much more generic, and hides implementation details
much more thoroughly, while keeping the same API / wrappers.

To do this we have to sub-class Poco::Logger to get access to
its generic 'log' method instead of a mess of in-line wrappers,
this lets us avoid lots of code.

Change-Id: I541d3aef49f99ce582655c5102a0041bf84cd56a
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-24 10:59:51 +01:00
Michael Meeks
1516ca22cb bgsave: set state to unmodified on successful background save.
There is a race here, clearly if you type while a background
save is ongoing - but this is far better than leaving the
document apparently unmodified.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ie5e3e692294e48ad887481af2e0906092830f265
2024-04-24 09:24:34 +01:00
Michael Meeks
52f00be490 bgsave: ensure that a bgsave process doesn't have a tile queue.
Otherwise we go on doing work that the parent kit process should do eg.

[ kitbgsv_006_002 ] TRC  ToMaster-8f9: saveDocumentBackground returns succesful start.| kit/ChildSession.cpp:887
[ kitbgsv_006_002 ] TRC  Calling paintPartTile(0x4c0716f0)| common/RenderTiles.hpp:130

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I257f61d05d8a0da0c8eb9d8c60e502da66dc8cdd
2024-04-20 16:08:38 +01:00
Michael Meeks
5f1387b6db bgsave: various documentation cleanups & a missing const.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I55636718815adc1b294219304e7fd060f88f9c15
2024-04-18 19:48:35 +01:00
Michael Meeks
33037c3a65 bgsave: don't try to save twice in the same background process.
Get the polarity of the check on whether we succeeded to spawn a
background save process right, so we don't spawn two saves
concurrently, and block input processing in the wrong place.

Also a few obvious sillies fixed, and some more assertions for
good measure; was not good:

[ kitbgsv_001_001 ] TRC  ToMaster-002: Finished synchronous background saving ...| kit/ChildSession.cpp:882
[ kitbgsv_001_001 ] TRC  ToMaster-002: saveDocumentBackground returns succesful start.| kit/ChildSession.cpp:887
[ kitbgsv_001_001 ] TRC  Document - input processing now: disabled was enabled| kit/Kit.cpp:2095
[ kitbgsv_001_001 ] TRC  ToMaster-002: uno command .uno:Save {... notify: true| kit/ChildSession.cpp:1944

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ie06a74a538bdd5038ca9c94b422f27cff9e4a82e
2024-04-18 19:48:35 +01:00
Michael Meeks
f845ac08af bgsave: have a single source for InputProcessing enable & disable.
Somehow this state can get confused in a bgsave process:

   Kit Document:
         ...
         inputProcessingEnabled: false
         ...
   SocketPoll:
     Poll [kit] with 1 socket - wakeup rfd: 39 wfd: 45
             fd        events        rbuffered        wbuffered        rtotal        wtotal
             52        0x1        process             0             0         r:    825

'process' should read 'ignore' for disabled input.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I787eebe6fda3ae1b527d7605b8813fa764e81890
2024-04-18 17:19:19 +01:00
Michael Meeks
d1fe734ed2 bgsave: Don't busy-spin when input processing is blocked.
It seems we've had input processing turned off more than expected
recently, if we have an event or callback in the queue, it seems
we set our timeout to zero.

Since we would not be processing input this would never change,
and we would just busy spin indefinitely; just lots of:

[ kitbgsv_001_001 ] TRC  ppoll start, timeoutMicroS: 0 size 1| net/Socket.cpp:404

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ib374c6ce12d467eba602c7eaee99cc3940ffe681
2024-04-18 13:49:27 +01:00
Michael Meeks
8b0b0239ae bgsave: improve input processing trace debugging.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I53ff0809248f010b5a569ef83d8d02f3287a77b5
2024-04-18 13:49:27 +01:00
Pranam Lashkari
50963aadf2 uno: .uno:SetDocumentProperties: add file path parameter
problem:
in LOK/online to support async save, files in jails may have
different extensions (ie: .upload, .uploading)
this caused problem to detect files as original file name may not exist.
As result property like file size were always set to 0

chronology of events:
1. File is saved normally with existing name
2. After saved we make it ready for upload and add extension .upload in "renameForUpload"(kit/ChildSession.cpp)
3  We change to .uploading extension when we are uploading (DocumentBroker::handleSaveResponse, DocumentBroker::uploadAfterLoadingTemplate)

Signed-off-by: Pranam Lashkari <lpranam@collabora.com>
Change-Id: Ibda40b0c134ef6baef9edb0427b3c56340924858
2024-04-18 10:07:13 +01:00
Caolán McNamara
ec9c6babcb on joining an existing session calc/writer can use new users spell preferences
but impress/draw should continue to use the existing document
preference. Writer and Calc have spell as a pre-view feature, while
impress/draw still has it per document.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ieb8c307b13f2ee010e09058a1ba63c7808637487
2024-04-18 08:46:46 +02:00
Michael Meeks
eceb1db8ec bgsave: remove closed sockets from Kit's SocketPoll.
Hard closing without shutdown is necessary, but we continued to poll
and read on an fd that would be re-used to open eg. a ZIP file:

[ kitbgsv_007_001 ] TRC  #19: Incoming WebSocket data of 13522 bytes: 50 4B 03 04 14 00 00 08  00 00 29 9C 90 58 33 26  AC A8 2F 00 00 00 2F 00  00 00 08 00 00 00 6D 69  | PK........)..X3&../.../.......m"
...
[ kitbgsv_007_001 ] ERR  #19: An unfragmented message or the first fragment of a fragmented message must have the opcode different than 0| net/WebSocketHandler.hpp:452

which would then close the file unhelpfully.

Not removing the socketHandler when cleaning up means that
we trigger the ForKit's ServerWSHandler::onDisconnect which
SigUtil::setTerminationFlag() causing all 2nd kit processes
to expire on start.

We also want to ensure that we update the thread-id of the last
forkit process before we start removing sockets and checking
thread-ids.

We want to get rid of the parent process' sockets we inherited
but don't need very cleanly post fork.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I82966f4421fc96df552fd50cf81c8b0bc92b9bbb
2024-04-17 08:35:30 +01:00
Michael Meeks
030acb1a85 bgsave: Add SLEEPBACKGROUNDFORDEBUGGER environment variable.
Change-Id: I7284d03ddcd79a0848d1cca9b219e2ef96548511
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-17 08:35:30 +01:00
Michael Meeks
af749c2237 bgsave: rename parameter to background, and add setting.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ic49ec5715682b71461b49741d022fc7149aa5a13
2024-04-16 16:43:52 +01:00
Michael Meeks
2d018d38a5 bgsave: add unit testing hooks for after bgsave fork & pre exit.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I35a2ec185762138dc85db39df3e7644c60acfddc
2024-04-16 16:43:52 +01:00
Michael Meeks
5dbe4fe02f bgsave: warn and fail on unexpected receipt of .uno:Save.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I097320d8b668c1bfae9052e9edc5a4df9c7c0bcf
2024-04-16 16:43:52 +01:00
Michael Meeks
a8102212c6 bgsave: ensure kit processes die when their parents do.
Potentially zombie / badly behaving kits should be taken down
by the kernel, and this lets us continue our cleanup by killing
just the parent process.

Change-Id: I1e81f41cded0c67b72622f8ed88602daf427238c
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-16 15:07:18 +01:00
Caolán McNamara
85fc0fd841 cid#360688 COPY_INSTEAD_OF_MOVE
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I426317d77e437d024ad2fb0133cf8eaa5c91e8c6
2024-04-16 15:19:35 +02:00
Michael Meeks
0b3b27cb72 bgsave: switch to using a 'save' command to the kit.
Also pass 'autosave' status in a more conventional way.

Ideally we would split 'save' away from a ChildSession / ClientSession
and have this on DocumentBroker / Document - for the future.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I39a6caf8b17fa2fc2d940ae0d11bdc2d4da20b6c
2024-04-15 11:06:22 +01:00
Michael Meeks
76230b6b4b SigUtil now associates all commands with a session anyway.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I9098f7137d81bbc63e2449fbc8ecc53b3f1e7e7e
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-15 11:06:22 +01:00
Caolán McNamara
bae0a91082 use SigUtil::setUserSignals before creating SocketPoll
The SocketPoll ctor which may, depending on COOL_WATCHDOG env variable,
want to override the SIG2 handler so set user signal handlers before
that otherwise tthat choice is overwritten

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I305570ab8becb41f0696e60908c1ca26fd9ba14a
2024-04-13 20:07:16 +01:00
Ashod Nakashian
d5f865d534 wsd: remove unused eof from the TileQueue
Change-Id: Ibb1ad403272554202b69f4c94f3b6f50c8db2379
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-04-11 08:31:24 +01:00
Michael Meeks
144b701453 cool#8703 - Drop random node creation and rely on inherited fd.
Re-using an inherited file descriptor to /dev/urandom frees us
from problems with mount options including 'nodev' and removes a
capability from the set we need.

Change-Id: I70337e923f802d7efbd3159c11a4e39f6529b6e6
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-07 12:11:48 +02:00
Michael Meeks
e27a095775 bgsave: initial implementation (gated on COOL_BGSAVE=1)
Initial background save implementation from the Kit perspective.

To do a background save we:

   1. join known threads - we can't fork with >1 thread.
   2. check all is well: one thread, nothing unusual...
   3. create a socketpair to communicate with the child
   4. fork
   5. child: cleanup duplicated sockets
   6. child: setup LOK to not damage our shared file-system
   7. child: save
   8. child: report status back to parent & _Exit

There is still a substantial TODO, but this can be built on.

Change-Id: Ibf2c492372e2b5133932773e230ad05e18521794
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-05 17:34:43 +01:00
Michael Meeks
ae9fee7f5a bgsave: add WebSocketHandlers for Kit to chat with its forked child.
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I224f120b9aae347c39d5c1471e2e74d8149c87f1
2024-04-05 17:34:43 +01:00
Patrick Luby
52a039e832 If there are no open documents, wait until one is added
Remove any stale elements from KitSocketPoll::KSPolls and
block until an element is added to KitSocketPoll::KSPolls

Signed-off-by: Patrick Luby <guibomacdev@gmail.com>
Change-Id: I25726171ef28d9107772f7665dd3cbb467e364e5
2024-04-05 09:02:13 -04:00
Caolán McNamara
bb7cd9f357 After fork we don't have a watchdog thread
So watchdog won't fire for a stalling kit.

After a fork the child has only one thread, but a copy of the watchdog
object.

Stop the watchdog thread before fork, let the child discard its copy of
the watchdog that is now in a discardable state.

And allow it to create a new one on the next SocketPoll ctor.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I7dc166dca3996401fbdc20cd7643f944662454c8
2024-04-05 10:50:33 +01:00
Pranam Lashkari
f29fe2a197 kit: set creation date when online creates file from template
problem:
when online created file using WOPI clients, creation dates were never set.
in online files are created using templates, even empty files are created using
an empty template

Signed-off-by: Pranam Lashkari <lpranam@collabora.com>
Change-Id: I312d7af17bf322ca55b6e20bba344e4fc6628e66
2024-04-03 13:26:24 +01:00