fix(networkjobs): move processEvents after reply reads in LsColJob - #10093
Conversation
|
/backport to stable-33.0 |
48b97f9 to
653f0b3
Compare
|
/backport to stable-4.0 |
There was a problem hiding this comment.
Pull request overview
This PR fixes a reproducible crash in LsColJob::finished() caused by calling QCoreApplication::processEvents() before reading from the QNetworkReply, which could drain a pending deleteLater() and null the underlying QPointer.
Changes:
- Move
QCoreApplication::processEvents(..., 100)to run after allreply()reads/accesses inLsColJob::finished(). - Add a regression test that forces a
QNetworkReplyto scheduledeleteLater()such that it can be processed duringprocessEvents(), ensuring no crash. - Add a “happy path” test to ensure normal listing/sync behavior remains unchanged.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/libsync/networkjobs.cpp |
Reorders processEvents() to avoid reply() use-after-free/null deref during LsColJob::finished(). |
test/testremotediscovery.cpp |
Adds regression + sanity tests covering reply deletion during processEvents() and normal listing behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LsColJob::finished() called QCoreApplication::processEvents() before reading the reply. A pending deleteLater() processed during that call would zero the QPointer<QNetworkReply>, causing a SIGSEGV on the subsequent reply()->request().url().path() access. Fix: move processEvents to after all reply() accesses so the pointer cannot be invalidated before it is used. Signed-off-by: Camila Ayres <[email protected]>
653f0b3 to
8925d0a
Compare
|
Artifact containing the AppImage: nextcloud-appimage-pr-10093.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
Resolves
Users on 4.0.9/4.0.10 started hitting a SIGSEGV at reply()->request().url().path() inside LsColJob::finished(). Crash address 0x8 (null pointer + 8 bytes) — the QPointer was zeroed by the time that line ran.
Summay
e9a0dbd (Jan 2024) added a QCoreApplication::processEvents() call inside LsColJob::finished() for UI responsiveness, placing it before the reply was read. The code even warned about this in a comment, but left the reply() access after processEvents() unguarded.
A pending deleteLater() on the reply — triggered by the network timeout timer — can be drained inside that processEvents() call, zeroing the QPointer. The function then crashes reading from null.
ee899a8 (March 2026) flipped enableTimeout = false → true, re-enabling the request timeout feature. With the timer active again, timeouts fire regularly during the 100 ms processEvents() window, making the race consistently reproducible in production.
This PR moves processEvents() to after all reply() accesses. The parse and signal emissions complete while the pointer is still valid. processEvents() still runs for UI responsiveness, just after parsing instead of before it.
Tests
Added to testremotediscovery:
Checklist
AI (if applicable)