Skip to content

fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder.#9813

Merged
camilasan merged 5 commits intomasterfrom
bugfix/unloadfolder
Apr 9, 2026
Merged

fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder.#9813
camilasan merged 5 commits intomasterfrom
bugfix/unloadfolder

Conversation

@camilasan
Copy link
Copy Markdown
Member

@camilasan camilasan commented Apr 8, 2026

Attempt to fix crash when exiting the client (on a computer reboot):

  • Disconnect Folder::canSyncChanged when unloading a folder to avoid callbacks after unload.
  • Remove doWait from broadcastMessage in slotUnregisterPath.
    Passing doWait=true caused socket->waitForBytesWritten(1000) to emit a signal.
    During shutdown, the signal could trigger slotUnregisterPath,
    corrupting _registeredAliases and crashing with EXC_BAD_ACCESS.
    Removing doWait eliminates the nested event loop and the re-entrancy.

ℹ️ 🤖 I did use Claude to analyse why the crash might be happening.

@camilasan camilasan requested a review from Copilot April 8, 2026 14:11
@camilasan camilasan marked this pull request as ready for review April 8, 2026 14:12
@camilasan camilasan changed the title fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder. wip: fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder. Apr 8, 2026
@camilasan camilasan marked this pull request as draft April 8, 2026 14:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR fixes stale signal connections when unloading a folder by disconnecting Folder::canSyncChanged from FolderMan, and refactors related path registration/unregistration logic for readability.

Changes:

  • Disconnect Folder::canSyncChanged when unloading a folder to avoid callbacks after unload.
  • Refactor slotFolderCanSyncChanged() local variable naming/const usage.
  • Refactor SocketApi::slotUnregisterPath() control flow and message building.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/gui/socketapi/socketapi.cpp Refactors unregister-path logic and message broadcast call.
src/gui/folderman.cpp Disconnects canSyncChanged on unload; minor refactor in slotFolderCanSyncChanged().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/gui/socketapi/socketapi.cpp
Comment thread src/gui/socketapi/socketapi.cpp Outdated
@camilasan camilasan force-pushed the bugfix/unloadfolder branch 2 times, most recently from f79fe21 to bfc72c3 Compare April 8, 2026 18:04
@camilasan camilasan marked this pull request as ready for review April 8, 2026 18:05
@camilasan camilasan changed the title wip: fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder. fix(folderman): disconnect slotFolderCanSyncChanged when unloading folder. Apr 8, 2026
@camilasan camilasan force-pushed the bugfix/unloadfolder branch 2 times, most recently from f30ead7 to 196e6b7 Compare April 8, 2026 18:21
@camilasan
Copy link
Copy Markdown
Member Author

/backport to stable-33.0

@camilasan
Copy link
Copy Markdown
Member Author

/backport to stable-4.0

…lder.

In case the account state changes during shutdown, the folder might not exist anymore.

Signed-off-by: Camila Ayres <hello@camilasan.com>
Signed-off-by: Camila Ayres <hello@camilasan.com>
…Path.

Passing doWait=true caused socket->waitForBytesWritten(1000) to emit a signal.
During shutdown, the signal could trigger slotUnregisterPath,
corrupting _registeredAliases and crashing with EXC_BAD_ACCESS.

Removing doWait eliminates the nested event loop and the re-entrancy.

Signed-off-by: Camila Ayres <hello@camilasan.com>
Signed-off-by: Camila Ayres <hello@camilasan.com>
Signed-off-by: Camila Ayres <hello@camilasan.com>
@camilasan camilasan force-pushed the bugfix/unloadfolder branch from 196e6b7 to 1a0bd15 Compare April 9, 2026 07:12
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

Artifact containing the AppImage: nextcloud-appimage-pr-9813.zip

Digest: sha256:c19cb4f8a52154c986e70169a561549cd0bdf83e46ebf2cb9321ed057ca784b2

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.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 9, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
72 New Code Smells (required ≤ 0)
E Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@camilasan camilasan merged commit 8cda11a into master Apr 9, 2026
22 of 23 checks passed
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.

3 participants