Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Await full sync stop before closing the app #2300

Merged
merged 1 commit into from
May 4, 2023

Conversation

taratatach
Copy link
Member

@taratatach taratatach commented May 4, 2023

When closing the application from a user request (via the "Quit"
button for example), we first stop the synchronization to avoid
corrupting data.

However, we were not waiting for the synchronization to be fully
stopped (i.e. it is an asynchronous process) before closing the
application.
We believe this might be the source of crashes on Windows.

Please make sure the following boxes are checked:

  • PR is not too big
  • it improves UX & DX in some way
  • it includes unit tests matching the implementation changes
  • it includes scenarios matching a new behaviour or has been manually tested
  • it includes relevant documentation

  When closing the application from a user request (via the "Quit"
  button for example), we first stop the synchronization to avoid
  corrupting data.

  However, we were not waiting for the synchronization to be fully
  stopped (i.e. it is an asynchronous process) before closing the
  application.
  We believe this might be the source of crashes on Windows.
@taratatach taratatach self-assigned this May 4, 2023
@taratatach taratatach merged commit 0901d11 into master May 4, 2023
@taratatach taratatach deleted the fix/wait-for-full-sync-stop-before-closing-app branch May 4, 2023 10:35
taratatach added a commit that referenced this pull request May 9, 2023
  This reverts commit 0901d11.

  By awaiting the full sync stop, we prevent quitting the app during a
  synchronisation or a watcher run while these can take a very long time
  (especially the first scan of local and remote changes).

  We'll bring back this behavior once we have a way to cancel the
  on-going sync and watch runs.
taratatach added a commit that referenced this pull request May 9, 2023
)

This reverts commit 0901d11.

By awaiting the full sync stop, we prevent quitting the app during a
synchronisation or a watcher run while these can take a very long time
(especially the first scan of local and remote changes).

We'll bring back this behavior once we have a way to cancel the
on-going sync and watch runs.
@taratatach taratatach restored the fix/wait-for-full-sync-stop-before-closing-app branch May 23, 2023 14:41
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