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.

Sign up for GitHub

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

Jump to bottom

Fix: explicitly cancel MVC thread futures for version monitors #4733

Merged
merged 4 commits into from
Apr 30, 2021
Merged

Fix: explicitly cancel MVC thread futures for version monitors #4733

merged 4 commits into from
Apr 30, 2021

Conversation

sfc-gh-abeamon
Copy link
Collaborator

@sfc-gh-abeamon sfc-gh-abeamon commented Apr 30, 2021 •
edited
Loading

Destruction of the thread futures for the protocol version monitors does not cancel the monitors. This explicitly calls cancel on the thread futures, which cancel the underlying activity.

This also fixes some issues that arose when the future returned a cancellation error.

  • Passed 10K correctness tests
  • Passed fdb_c_unit_tests
  • Passed 10K binding tester runs
  • Passed multithreaded_client.py tests

This also passed the same tests run in #4667.

Code-Reviewer Section

The general guidelines can be found here.

Please check each of the following things and check all boxes before accepting a PR.

  • The PR has a description, explaining both the problem and the solution.
  • The description mentions which forms of testing were done and the testing seems reasonable.
  • Every function/class/actor that was touched is reasonably well documented.

sfc-gh-anoyes
sfc-gh-anoyes previously approved these changes Apr 30, 2021
View reviewed changes
@sfc-gh-abeamon
Add cancellation of the protocol monitor when the database is destroy…
cb3d2bf 
…ed. Avoid using any state when cancelled. Fix race between setting up the protocol version monitor and destroying the database.
@sfc-gh-abeamon sfc-gh-abeamon merged commit e630090 into apple:master Apr 30, 2021
@sfc-gh-abeamon sfc-gh-abeamon mentioned this pull request May 1, 2021
Merged
5 tasks
@sfc-gh-abeamon sfc-gh-abeamon deleted the fix-mvc-thread-future-cancellation branch June 3, 2021 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-anoyes sfc-gh-anoyes sfc-gh-anoyes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sfc-gh-abeamon @sfc-gh-anoyes