Commit graph

1099 commits

Author SHA1 Message Date
Michael Meeks
0c7cb448ca cypress and C++ tests: enable full trace logging to keep timing the same.
It appears that both cypress and the C++ WOPI tests are extremely
timing sensitive, and fixing this should not hold up merging the
logging improvements. So for now don't disable logging in these modes.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I43e8397263e6960b668a29e7ad67f45394c52b52
2024-05-14 18:37:52 +02:00
Michael Meeks
9b6ab4c601 Logging: add Area parameter and new LOGA_ macros to annotate areas.
Add logging.disabled_areas setting to coolwsd.xml with some sensible
things to ignore unless they are warnings/errors.

Kit code duplication around logging is grim; but not fixed in this
commit.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I36bebb2b3c8d64a814d7b10c167d582de0baf4e5
2024-05-14 18:37:52 +02:00
Caolán McNamara
c46df319ec cid#366099 Uninitialized pointer field
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I30949ae9a944a1c254c24a49519b22cf6202a14f
2024-05-13 08:44:03 +02:00
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
fff63bec45 Forkit needs to wakeup to waitpid processes.
This should cleanup jails more quickly.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I2f7c7753614f0845a4d81d69334834047d661e41
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
Ashod Nakashian
6f725087a1 mount: remove test-mount directory
Change-Id: Iff128a22ac7d0fa3e6514e62e1f4ea92a60f052b
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-05-09 15:32:26 +02:00
Ashod Nakashian
b33f822871 wsd: mount: safeRemoveDir will not error when unmount fails
safeRemoveDir is tries to unmount blindly, so there
is little point in erroring when the directory
in question isn't a mount-point at all.

Change-Id: I6db0fd9406493060ce52a69f7d935b0958e2d2be
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-05-09 15:32:26 +02:00
Ashod Nakashian
dac5c0341f mount: avoid hard-coding
Change-Id: I6857510a9d0442cc357886b453d369f1e4b8f53f
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-05-09 15:32:26 +02: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
aacc957f23 MessageQueue: squash into TileQueue.
Avoids unhelpful virtual methods which are not used anywhere, simplifies
several code-paths, improves readability, and perhaps performance.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I7528fab77698546545bf81d7ccacdda9a0002833
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
7f6b59b180 bgsave: improve unit tests.
Force background saving in the config for this test.

Use stamp files to force sequencing between Unit test and Kit.

Change-Id: Ia2c60c3dcfdad87c9c9754e8f20a3c36dbcf74d5
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-05-08 16:42:29 +01:00
Caolán McNamara
1385b0d086 cid#365222 COPY_INSTEAD_OF_MOVE
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Idf287528f5e17670bef3a7524e8cefc91ae23ffc
2024-05-07 15:59:55 +02:00
Caolán McNamara
ec8cc5aea8 cid#365225 COPY_INSTEAD_OF_MOVE
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I9c7e1b105d6cb10ef312b7a824de8cc922a83929
2024-05-07 15:59:55 +02:00
Hubert Figuière
22e789a420 impress: restrict presentation in readonly mode
When export is disabled or watermarks are enabled, presentation in readonly mode
is disabled.
This is checked in the frontend by the WOPI property `DisablePresentation`
that is synthesized in the wsd. Also check when calling the presentation
command.
WOPIFileInfo::getWatermarkText() is stubbed on mobile.

Signed-off-by: Hubert Figuière <hub@collabora.com>
Change-Id: I4f7aff9f670f7523dfcf396f6009a272df9d5af8
2024-05-07 13:21:04 +01: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
9d67fe24d2 comments: remove obsolete thread related comments.
Change-Id: I9d31e2614c7ea6edf47b92f25edd60748dd17493
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
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
038e278b03 bump watchdog pings from 20ms to 50ms
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ib1add9a1e392e96ad9f92feffd518b92aeb6a81c
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
a866719881 test: abort a test whose kit has a segfault by default.
Otherwise forkit tends to loop aggressively re-starting that for the
duration of the test wastefully.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: Ia1c684a5d995f54f29290c9631b1ee14266445d7
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
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
2d1865a8f6 logging: remove now unused StreamLogger.
Change-Id: Ic8a28dcc97aa0b17d2cb8efa99c42f7791486a15
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-24 10:59:51 +01: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
Henry Castro
722a2e57d2 wsd: simplify LOG_MESSAGE to the logging framework
Change-Id: I997855e586783451a556cefe2860f9384585045a
Signed-off-by: Henry Castro <hcastro@collabora.com>
2024-04-24 10:59:51 +01:00
Henry Castro
1271ee3e7f wsd: remove "PRIO" parameter to the logging framework
remove POCO dependency enumerator value.

Change-Id: I2abccd591374db96be81932d25b59fb0b10bac69
Signed-off-by: Henry Castro <hcastro@collabora.com>
2024-04-24 10:59:51 +01:00
Henry Castro
335e29abf7 wsd: add 'name' parameter to the logging framework
Change-Id: Ic8cc82e06200d2c2c8f47e27fd5e3225ca7f6202
Signed-off-by: Henry Castro <hcastro@collabora.com>
2024-04-24 10:59:51 +01:00
Michael Meeks
3c6d158615 A more plausible default for _isWriteable.
In all WOPI cases this is overridden by CheckFileInfo etc. but for
legacy direct file:/// usage this state can still be seen from
unit tests it seems.

cf. commit 115b9cf2ab

dump_state() output:

 Document broker sessions [1]
		id: 001
		name: ToClient-001
...
		isWritable: false
		isReadOnly: false

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I19792b1dfdb6f5126a4d119a4d001a06bc507fb7
2024-04-22 08:50:38 +01:00
Caolán McNamara
aba11a6d73 fix test failure with hardening flags enabled
constexpr std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](size_type) [with _Tp = char; _Alloc = std::allocator<char>; reference = char&; size_type = long unsigned int]: Assertion '__n < this->size()' failed.

Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: I182678a1b4fcdf9e9481779f4cd1cf0ec55b2cb1
2024-04-18 21:21:04 +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
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
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
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
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
Aron Budea
246e87fea2 wsd: Unconditionally compile support-key-enabled code paths
Signed-off-by: Aron Budea <aron.budea@collabora.com>
Change-Id: Iec477c223f2dc75e0be8b472c8439ca9311d6aae
2024-04-14 01:23:00 +01:00
Ashod Nakashian
fc4de6db18 wsd: redunce string concatenation churn
Change-Id: Ic14a14b6fb69346d9f3e3638620ec35b7c2eb6c1
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-04-11 08:31:24 +01:00
Caolán McNamara
ee807283d4 drop newly unused HAVE_GETENTROPY and HAVE_SYS_RANDOM_H
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Iccfa44f56fb0d3b185df3c9c8457cf19d42f85f1
2024-04-07 12:11:48 +02: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
7311352dd6 Random re-work: keep a file-descriptor open to /dev/urandom
This should be inherited by forked children, and kept alive at all
times. If we have it already open everywhere, there seems little
benefit to the getrandom / getentropy system calls.

Change-Id: I5d58f7216c65febd161cbd78c24308d9192830ee
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-04-07 12:11:48 +02: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
Ashod Nakashian
062574341f wsd: move wopi stub/dummy interface to MobileApp.hpp
Change-Id: I58ac7467c4e74059e4de08c914546614ed6fe883
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>
2024-04-03 14:26:28 +01:00
Michael Meeks
df343fc5fa Watchdog: joinThread should tolerate not having a thread yet.
Change-Id: I9a0fe85d22dbe62c9be492e1c71dbb41f9253303
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-03-29 13:04:09 +00:00
Michael Meeks
588aabb7c3 Watchdog: re-direct USR2 when enabled to snapshot a late thread.
By tracking the thread-id, we can deliver a SIGUSR2 to the right
thread at the right time; this avoids perf polling our uninteresting
watchdog thread.

In that thread use Caolan's suitably obscure futimestat system-call,
so that we can record based on that to see only slow things:

perf record -e syscalls:sys_enter_futimesat -ag --call-graph dwarf,65528

Change-Id: Iad05d8589fdc9541a7d0599f63625d2cde5fdf89
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-03-29 10:35:32 +00:00
Michael Meeks
40f4977792 forkit: quicker zombie reaping by handling SIGCHLD.
Should launch new children more quickly on child death,
as well as getting to a waitpid and cleaning the zombies
faster too.

Change-Id: I06c36f63ac7ff52c407f739f1ce10d5e680fb82f
Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
2024-03-26 20:16:08 +00:00
Michael Meeks
fd77301ebb polls: switch compiled in delays to 60 seconds.
Forkit forking children is done in response to socket messages,
and parent process death should kill us too.

In general if we are relying on a poll to spin to achieve
something, we have a performance bug; this should exacerbate them
to flush them out.

Signed-off-by: Michael Meeks <michael.meeks@collabora.com>
Change-Id: I60d1c3b3c2532bbd686a3d3cfdea10f2a541a19a
2024-03-25 08:18:35 +00:00
Jaume Pujantell
8921e19d84 reduce uses of MOBILEAPP on some files
Reduce the uses of MOBILEAPP conditionals by using the isMobileApp
function.

Signed-off-by: Jaume Pujantell <jaume.pujantell@collabora.com>
Change-Id: If541307fbc457b342674cc560b6c53454f3904cf
2024-03-20 09:13:00 +01:00