GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||
GeezerVison is showing github.com — Fix crash when stopping pipeline with stop token by jborean93 · Pull Request #28300 · PowerShell/PowerShell Conversation
Collaborator
PR SummaryThere is a race condition where The fix is to ignore PR ContextFixes: #28299 It also fixes a related problem where if the delegate registered by a caller could throw which is raised as an function Test-Stop { [CmdletBinding()] param() $null = $PSCmdlet.PipelineStopToken.Register([Action]{ }) Start-Sleep 30 } Test-Stop # press Ctrl+C
The fix is to ignore This should be considered for backporting as well. While it is a race condition, the issue from #28299 shows that it can crash the whole process. PR Checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsThere is a race condition where `PipelineProcess.Stop()` can be called after `Dispose()` has already disposed the token. This causes the `Stop()` call to raise `ObjectDisposedException` crashing the process. An explicitly registered action on `$PSCmdlet.PipelineStopToken.Register()` can also throw an exception which is not handled in the stop call causing a similar crash. The fix is to ignore `AggregateException` which contains any exceptions raised by the user defined delegates on the stop token and exit early if the token has been disposed. We exit early because `Dispose()` has already done the work and there is nothing left to stop.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Changes recommendedThe tests do not reliably exercise cancellation of a disposed token source or the AggregateException handler, leaving the original crash without dependable regression coverage. 2 open issuesWhat changed in this PRThis PR hardens pipeline cancellation to prevent shell crashes when stopping races with disposal or a registered stop-token callback throws, addressing #28299. Changes:
Review effort: Balanced
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure how this would be possible without adding test only hooks that lock the state to a particular set of steps. This would be very complicated and means we start testing an implementation detail. Ultimately this is a race condition that I don't believe we can really test against.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactionsUh oh!
There was an error while loading. Please reload this page.
Ilya (iSazonov)
approved these changes
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
Copilot code review
Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
At least 1 approving review is required to merge this pull request. Uh oh!
There was an error while loading. Please reload this page. AssigneesNo one assignedLabels
CL-General
ProjectsMilestoneNo milestoneDevelopmentSuccessfully merging this pull request may close these issues. pwsh 7.6 crashes with ObjectDisposedException in PipelineProcessor.Stop when Ctrl+C arrives as a native command endsUh oh!
There was an error while loading. Please reload this page. Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later. | ||||||||||||||||||||
| You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required. |