bookmarks: App's fetch-timer control test segfaults intermittently under CI load

[Yaraslaut]

ladder_bookmarks_tests segfaulted once in CI on the control half of the
stopBackgroundJobs() pair. It has not reproduced since, so this is filed with
the evidence that exists rather than a recipe.

What happened

Linux / all optional features (clang), ctest:

1524/2475 Test #1524: App's fetch timer really does fire under a pumping loop (the control for stopBackgroundJobs) ...***Exception: SegFault  0.22 sec

The following tests FAILED:
	1524 - App's fetch timer really does fire under a pumping loop (the control for stopBackgroundJobs) (SEGFAULT) ladder-bookmarks
Errors while running CTest

99% tests passed, 1 tests failed out of 1524 — the segfault was the only failure
in the run.

Verification status

Reproduced: once, in CI, not on demand.

• Revision fb3f4c9a (branch ci/self-hosted-linux-runner), run
  33603372999 attempt 1,
  self-hosted runner morph-docker-1, started 2026-09-02T07:40:22Z.
• The runner is a Docker container pinned to 2 processors (--cpuset-cpus) with
  a 6 GiB cap, one of five such containers compiling concurrently on a
  12-processor host. The machine was heavily loaded, which is the one
  environmental difference worth noting against every green run of this test.

Not reproduced, four ways:

• 40/40 passes running that single test in a loop locally, same revision, on a
  binary built through the compiler cache.
• The Application ladder / ASan+UBSan leg of the same CI run executed the
  same test and passed: 208/976 Test #1512: ... Passed 0.17 sec, and the leg
  finished 100% tests passed, 0 tests failed out of 976. So neither ASan nor
  UBSan flagged anything on the path that crashed.
• The rerun of the failed job (attempt 2) was green, 26/26.
• Three earlier full runs on adjacent revisions were green.

Not verified: no core dump or backtrace was captured — ctest reports only
SegFault, and the container is gone. Everything below the next heading is read
from the source, not observed.

Mechanism — inferred from reading, NOT reproduced

examples/bookmarks/tests/test_app.cpp:370 pumps until a timer-driven pass has
dispatched, pumps until fetchInFlight() is false, then lets App destruct at
scope exit.

The parts that are already defensive, and so are probably not it:

• App::~App() (examples/bookmarks/src/app/app.cpp:167) calls
  stopBackgroundJobs() first, with a comment saying exactly why.
• The in-flight counter is a shared_ptr<atomic> captured by value into
  .then()/.onError(), explicitly so a late completion outliving the App
  can still decrement it safely.

The part that does not have that protection: the per-pass handler at
examples/bookmarks/src/app/app.cpp:216 is built over _fetchBridge and
&_fetchExecutor, both App members:

auto handler = std::make_shared<::morph::bridge::BridgeHandler<BookmarkModel>>(_fetchBridge, &_fetchExecutor);

The shared_ptr keeps the handler alive past ~App, but not the executor it
points at. If any queued delivery reaches that handler after ~App has run,
it touches a destroyed member. fetchInFlight() == false says every dispatch
settled its counter; it is not obviously the same statement as "no queued event
referencing _fetchExecutor remains", and the test destroys the App
immediately on that signal.

This is a hypothesis that fits the crash site and the load-sensitivity. It is
not evidence, and it may be wrong.

What would change the verdict

• Close if the mechanism above is disproved (e.g. the executor provably
  cannot be reached after ~App) and the crash is traced elsewhere, or if it is
  fixed and a stress loop under artificial load stays green.
• Reopen / raise severity if it recurs, especially on a GitHub-hosted runner
  or an unloaded machine — that would make it a plain lifetime bug rather than
  one that needs contention to surface.
• Worth trying to force before doing either: run this test under heavy CPU
  contention in a loop, and/or shorten kFastFetchInterval, to see whether it
  becomes reproducible on demand. A version of it that reproduces is worth more
  than this report.

The self-hosted runners make this class of failure more likely to be seen,
because a 2-processor container under load stretches exactly the windows a
same-thread event-loop test relies on staying closed.

[Yaraslaut]

Triage: rescope

The crash is real and the report is honest about its own limits. The
mechanism it proposes is disproved, and two of its three "what would change
the verdict" steps have now been run and do not work. The parts worth keeping
are the observation and the instinct behind it, not the diagnosis.

The proposed mechanism does not hold

The shared_ptr keeps the handler alive past ~App, but not the executor
it points at. If any queued delivery reaches that handler after ~App has
run, it touches a destroyed member.

Both members it names are explicitly protected, and both protections are
deliberate:

• _fetchExecutor is declared first in App (app.hpp:183), so it is
  destroyed last — after ~_pool has joined its worker threads. The
  20-line comment above it (app.hpp:166-182) says exactly this, and
  pastebin's App carries the identical comment recording that this precise
  bug was already found and fixed there.
• _fetchBridge cannot be touched by ~BridgeHandler: that destructor
  (bridge.hpp:1949) checks a CallbackToken and deregisters nothing when the
  Bridge is already gone.

I checked two alternative lifetime paths of my own; both are also refuted by
reading:

• RemoteServer::handleImpl captures self = shared_from_this() into the pool
  task (remote.hpp:349,369), so the server cannot die under a running worker.
• SimulatedRemoteBackend::execute's reply lambda captures only state and
  deser (remote.hpp:2153) — never the backend — so the reply path never
  touches the Bridge.

Neither suggested next step reproduces it

• Stress under contention: 0/200. Control test pinned to 2 CPUs
  (taskset -c 0,1) with 16 spinners saturating a 12-CPU host — the closest
  local analogue of the morph-docker-1 container. 200 consecutive runs, zero
  failures. Consistent with the report's own 40/40.

• ThreadSanitizer cannot adjudicate this. The ladder is never built under
  TSan (ci.yml:506 pins the only TSan leg to
  -DMORPH_LADDER_RUNGS=kanban), so I built bookmarks under clang-tsan
  locally. It reports 720 data races over the suite — but they are the
  unfalsifiable kind Kanban/ThreadSanitizer still races after #127's fix: Qt-internal QCallableObject reuse, kanban-tsan job's own premise is wrong #128 already characterised. Every one I inspected has the
  same shape:

  Read  of size 8 ... by main thread:
    #2  morph::qt::QtExecutor::post(...)::'lambda'()::operator()()  qt_executor.hpp:86
    #8  QObject::event(QEvent*)                                     libQt6Core.so.6
  Previous write of size 8 ... by thread T1:
    #4  morph::qt::QtExecutor::post(std::function<void()>)          qt_executor.hpp:83
    #5  morph::async::detail::CompletionState<...>::setValue(...)   completion.hpp:79
    #17 morph::exec::ThreadPoolExecutor::loop()                     executor.hpp:106
  

  The happens-before edge is Qt's own event-queue mutex, and
  nm -DC /usr/lib/libQt6Core.so.6 | grep -c __tsan → 0. That is exactly
  Kanban/ThreadSanitizer still races after #127's fix: Qt-internal QCallableObject reuse, kanban-tsan job's own premise is wrong #128's closing finding: "TSan cannot see the happens-before edges established
  inside a prebuilt, uninstrumented QtCore ... every warning whose ordering
  depends on Qt-internal synchronisation is unfalsifiable from outside an
  instrumented Qt build."

  (Instrumentation was verified non-vacuous, per rule 7: 618 __tsan symbols
  in the binary, -fsanitize=thread on app.cpp in compile_commands.json,
  and a two-thread positive control fires.)

What the report gets right, and the framing it should have

Two things it says are correct and worth more than the mechanism:

1. fetchInFlight() == false is genuinely not "no pool thread is still
   running." It only says every dispatch settled its counter. Only ~_pool's
   join establishes the stronger statement.
2. The green ASan+UBSan leg is not evidence. ASan does not detect data
   races, and — more directly — QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127 was a bug of exactly this class that
   manifested as a plain SIGSEGV with no sanitizer at all, and crashed two
   uninstrumented legs: Linux / clang-coverage and
   Linux / all optional features (clang) — the same leg that crashed
   here.

That points at the actual gap. #127's fix was QtExecutor's _alive token,
and app.hpp:178-181 states its residual limit precisely: _alive guards
delivery of an already-queued event, not a post() on a freed
IExecutor*. In App, that remaining hole is closed by member declaration
order and nothing else.

Nothing tests that ordering. There is no test in examples/bookmarks/tests/
or examples/pastebin/tests/ that would fail if the members were reordered into
their natural reading order. The suite would stay green and the bug would come
back exactly as #127 describes it: intermittently, under load, in whichever test
happened to be running. By rule 7 — would this still pass if the protection did
nothing? — yes, it would.

So the issue to build from is not "the handler touches a destroyed member." It
is: the one protection standing between App and a known, previously-shipped
class of segfault is an unenforced declaration order, and #391 is the kind of
crash its absence would produce.

What would change the verdict

• Confirm the mechanism if a run captures a core dump or backtrace, or if a
  reordering experiment (move _fetchExecutor to its natural position and
  stress) reproduces a segfault — that would establish that the ordering is what
  is holding, and that bookmarks: App's fetch-timer control test segfaults intermittently under CI load #391 is a different hole in it.
• Close if it never recurs and a guard against the reordering regression
  lands regardless — the guard is worth having on QtExecutor::post() heap-use-after-free when a nested Completion chain's post outlives the executor's teardown #127's evidence alone,
  independently of whether it explains this crash.
• Raise severity if it recurs on a GitHub-hosted runner or an unloaded
  machine, per the original report.

Not reproduced here; the crash remains unexplained. Labelling triage: rescope
rather than unverified because the premise is verified — there is a CI log —
and what needs rewriting is the diagnosis, not the observation.

[Yaraslaut]

Second occurrence — and this one is under GCC, not Clang

This was filed with the evidence that existed and deferred for having no
reproduction. It has now happened again, on an unrelated pull request, and the
new data point changes what can be said about it.

Run: Linux / all optional features (gcc), job 100546659510, on
lane/n-ladder at eec3d44d — a branch touching examples/ledger,
examples/kanban, examples/crm and two new testkit test files, and nothing
in examples/bookmarks, include/morph/'s executor, strand or timer code, or
gui/event_poller.cpp. ladder_bookmarks_tests is a separate binary from the
one this branch changes.

App's fetch timer really does fire under a pumping loop (the control for
stopBackgroundJobs)
examples/bookmarks/tests/test_app.cpp:370: FAILED:
  {Unknown expression after the reported line}
due to a fatal error condition:
  SIGSEGV - Segmentation violation signal

1538 - App's fetch timer really does fire under a pumping loop
       (the control for stopBackgroundJobs) (SEGFAULT) ladder-bookmarks
99% tests passed, 1 tests failed out of 1538

Byte-identical in shape to the original report: same test, same file, same
test_app.cpp:370, same lone failure in an otherwise green run.

What the second occurrence rules out

• Not compiler-specific. The first hit was Linux / all optional features (clang); this is the gcc leg of the same matrix. A clang miscompile is no
  longer a candidate.
• Not a one-off. Two independent hits on unrelated revisions, weeks of green
  runs between them.
• Not caused by the change under test. Neither branch touched bookmarks, the
  timer path, or the executor.

What it still does not give

An on-demand reproduction. Both hits were on loaded CI runners; the original
report recorded 40/40 passes locally, and the same run's ASan+UBSan leg executing
the same test and passing. That combination — a crash that ASan does not flag —
is itself informative and worth keeping in view: it points away from a plain
heap-use-after-free and toward a lifetime or teardown-ordering problem that only
manifests when the pumping loop and the timer interleave a particular way.

Why this should leave Deferred

It was deferred on the stated ground that there was no reproduction and therefore
nothing to act on. That ground is weaker now: the failure recurs across
compilers, it takes a whole CI leg down when it fires, and it has now blocked an
unrelated pull request, which is a cost this repository is paying rather than
merely a risk it is carrying.

The obvious next step no longer needs a reproduction to be worth doing: run that
single test under --repeat until-fail on a deliberately loaded machine, and
under TSan rather than ASan, since a teardown race is the shape the evidence
points at.

Not done here

I have not touched the test, the timer, or stopBackgroundJobs. This comment
only records the second data point and the argument for scheduling it.