-
Notifications
You must be signed in to change notification settings - Fork 29
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
[coverage] Fix isolate resumption after service disposal #1189
Conversation
Package publishing
Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation. |
PR HealthBreaking changes ✔️
Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. Coverage ✔️
This check for test coverage is informational (issues shown here will not fail the PR). API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
License Headers ✔️
All source files should start with a license header. Unrelated files missing license headers
|
Pull Request Test Coverage Report for Build 11716029981Details
💛 - Coveralls |
Revisions updated by `dart tools/rev_sdk_deps.dart`. dartdoc (https://github.com/dart-lang/dartdoc/compare/24c2a96..6bbd3d7): 6bbd3d7b 2024-11-14 Sam Rawlins Do not use SpecialClass to evaluate Enum or Interceptor (dart-lang/dartdoc#3928) dcc239a4 2024-11-12 Sam Rawlins Refactor special case for how canonical enclosing containers are calculated (dart-lang/dartdoc#3925) 80032309 2024-11-11 Sam Rawlins Ignore static issue with overridden field (dart-lang/dartdoc#3926) http (https://github.com/dart-lang/http/compare/03ced4d..2f954e1): 2f954e1 2024-11-11 Brian Quinlan Add macOS tests for `package:cupertino_http` (dart-lang/http#1403) shelf (https://github.com/dart-lang/shelf/compare/1a141c7..0bb44cb): 0bb44cb 2024-11-12 Devon Carew add direct links to the package issues (dart-lang/shelf#455) tools (https://github.com/dart-lang/tools/compare/66afa68..a6603a4): a6603a45 2024-11-11 Liam Appelbe [coverage] Fix isolate resumption after service disposal (dart-lang/tools#1189) Change-Id: I715f943ff69ae24b5126a7ddf883d31aed180632 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/395402 Reviewed-by: Konstantin Shcheglov <[email protected]> Auto-Submit: Devon Carew <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
The sequence of events that reproduces the bug is:
IsolatePausedListener._allNonMainIsolatesExited
is completed, soIsolatePausedListener.waitUntilAllExited
starts waiting onIsolatePausedListener._mainIsolatePaused
.IsolatePausedListener._mainIsolatePaused
is completed, soIsolatePausedListener.waitUntilAllExited
collects and resumes the main isolateIsolatePausedListener
tries to collect and resume them, causing the exception seen in the bug.Step 5 is the bit I didn't consider when writing the new flow. The fix is just to make
_allNonMainIsolatesExited
wait for main to pause (logically this future is now_allNonMainIsolatesExitedAndMainIsolatePaused
, but that's a bit verbose).Details:
_checkCompleted()
when main is paused (to fix a different edge case), so we're still going to complete the_allNonMainIsolatesExited
future even if all the non-main isolates exit before main pauses._runCallbackAndResume()
call now has to check if main's group has already been collected.Fixes #685