Skip to content

Conversation

@ChristopheLarchier
Copy link
Contributor

During a sync restart after a DataError, the localFileSystemObserverWorker continues to run and in case of AccessDenied, the handleAccessDeniedItem method is called which accesses the updateTree while it is destroyed/recreated by the sync restart.

@ChristopheLarchier ChristopheLarchier added this to the 3.7.9 milestone Nov 12, 2025
@ChristopheLarchier ChristopheLarchier requested a review from a team as a code owner November 12, 2025 09:55
@ChristopheLarchier ChristopheLarchier marked this pull request as draft November 12, 2025 11:05
@ChristopheLarchier ChristopheLarchier marked this pull request as ready for review November 12, 2025 13:35
@ChristopheLarchier ChristopheLarchier modified the milestones: 3.7.9, 3.7.10 Nov 13, 2025

class SYNCENGINE_EXPORT SyncPal : public std::enable_shared_from_this<SyncPal> {
public:
static std::recursive_mutex updateTreesMutex;
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not comfortable with the idea of a mutex being accessible anywhere in the code. This seems to have high risk of deadlock and/or impacting performance.
Moreover, it seems to me that only the running module should be accessing and modifying the update trees. Therefor, no concurrent access should occur. Maybe the algorithm is to be refactored somewhere instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LocalFileSystemObserverWorker updates the update trees and runs in parallel with the other modules:
LocalFileSystemObserverWorker::sendAccessDeniedError => SyncPal::handleAccessDeniedItem => UpdateTree::deleteNode

@ChristopheLarchier ChristopheLarchier marked this pull request as draft November 14, 2025 15:26
@ChristopheLarchier ChristopheLarchier marked this pull request as ready for review November 14, 2025 15:41
@sonarqubecloud
Copy link

@ChristopheLarchier ChristopheLarchier marked this pull request as draft November 17, 2025 07:34
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