Skip to content

fix(backend): add the missing Life guard to PreviewProvider - #9

Open
nocstah wants to merge 1 commit into
Percius04:masterfrom
nocstah:fix/previewprovider-lifetime-guard
Open

fix(backend): add the missing Life guard to PreviewProvider#9
nocstah wants to merge 1 commit into
Percius04:masterfrom
nocstah:fix/previewprovider-lifetime-guard

Conversation

@nocstah

@nocstah nocstah commented Aug 18, 2026

Copy link
Copy Markdown

f8ecfbd gave FileOperations, SearchWorker and ThumbnailProvider a Life{mutex,alive} guard so a pool worker can never touch this after the owner is destroyed. PreviewProvider has the same pattern and wasn't in that commit — it still has no guard and no destructor.

Both of its async entry points hand a raw this to the global pool and then deliver with QMetaObject::invokeMethod(this, ...):

  • requestText() — reads up to maxBytes and runs SyntaxHighlighter::highlight() on the worker, so the window is as wide as a large file's highlight pass
  • requestAudio()MediaInfo::extract() on the worker

invokeMethod() dereferences this to find its thread, so if the singleton dies while a worker is still in that read — app shutdown, or an xdg-desktop-portal picker answering and tearing the QML engine down — the worker reads freed memory.

Reproduction

Against f8ecfbd: 8 concurrent requestText() calls on a 560 KB highlightable file, provider destroyed mid-flight, freed block poisoned so a dangling read faults hard rather than silently reading stale-but-mapped memory.

3/3 runs SIGSEGV before, 0/3 after, at this frame (symbolized):

#0 QObject::thread()
#1 QMetaObject::invokeMethodImpl
#2 PreviewProvider::requestText(QString const&, int)::{lambda()#1}
#3 QRunnable::QGenericRunnable::Helper<...>::impl

AddressSanitizer is the better instrument here and is what you used for f8ecfbd; libasan wasn't available on my machine, hence the heap-poisoning harness. It reproduces the same frame I first saw in a real coredump — mine came from the FileChooser portal path (ThumbnailProvider, pre-f8ecfbd), which is what sent me looking at the sibling classes in the first place.

The change

Your own pattern from f8ecfbd, unchanged: Life{mutex,alive} in a shared_ptr that outlives the singleton, a destructor that marks it dead under the lock, and delivery that takes that same lock before touching this.

One deliberate difference from SearchWorker: the generation counters stay as plain members. They're only read inside the delivered lambda, on the object's own thread — unlike SearchWorker's directory walk, which polls its counter per entry from the worker and therefore genuinely needed the shared_ptr move.

Two files, +38/−2, no behaviour change on the happy path.

The P0 concurrency remediation (f8ecfbd) gave FileOperations, SearchWorker
and ThumbnailProvider a Life{mutex,alive} guard so a pool worker can never
touch `this` after the owner is destroyed. PreviewProvider has the same
pattern and was not part of that commit: it still has no guard and no
destructor.

Both of its async entry points hand a raw `this` to the global pool and then
deliver with QMetaObject::invokeMethod(this, ...):

  requestText()  - reads up to maxBytes and runs SyntaxHighlighter::highlight()
                   on the worker, so the window is as wide as a large file's
                   highlight pass.
  requestAudio() - MediaInfo::extract() on the worker.

invokeMethod() dereferences `this` to find its thread, so if the singleton
dies while a worker is still in that read (app shutdown, or an
xdg-desktop-portal picker answering and tearing the QML engine down), the
worker reads freed memory. Symbolized:

  #0 QObject::thread()
  Percius04#1 QMetaObject::invokeMethodImpl
  Percius04#2 PreviewProvider::requestText(QString const&, int)::{lambda()Percius04#1}
  Percius04#3 QRunnable::QGenericRunnable::Helper<...>::impl

Reproduced against f8ecfbd by firing 8 concurrent requestText() calls on a
560 KB highlightable file, destroying the provider mid-flight and poisoning
the freed block: 3/3 runs SIGSEGV at the frame above, 0/3 with this change.
(AddressSanitizer would be the better instrument, as used for f8ecfbd; libasan
was not available on this machine, hence the heap-poisoning harness, which
reproduces the same frame seen in a production coredump.)

The fix is f8ecfbd's own pattern, unchanged: Life{mutex,alive} held in a
shared_ptr that outlives the singleton, a destructor that marks it dead under
the lock, and delivery that takes that same lock before touching `this`. The
generation counters need no move here -- unlike SearchWorker's walk, they are
only read inside the delivered lambda, on the object's own thread.
@nocstah
nocstah force-pushed the fix/previewprovider-lifetime-guard branch from 7c86a8e to 02552e2 Compare August 18, 2026 04:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant