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

Cancelling static graph-based restore does not end the restore process #13140

Closed
jeffkl opened this issue Jan 8, 2024 · 0 comments · Fixed by NuGet/NuGet.Client#5573
Closed
Assignees
Labels
Area:RestoreStaticGraph Issues related to the Static Graph restore Priority:2 Issues for the current backlog. Type:Bug
Milestone

Comments

@jeffkl
Copy link
Contributor

jeffkl commented Jan 8, 2024

NuGet Product Used

MSBuild.exe, dotnet.exe

Product Version

All

Worked before?

No response

Impact

It bothers me. A fix would be nice

Repro Steps & Context

Static graph-based restore launches an external process and waits for it to return:

https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Build.Tasks/StaticGraphRestoreTaskBase.cs#L184

However, the code passes a cancellation token which is fired when the user cancels a build via CTRL+C. The catch code is there to return true so that cancelled builds don't "fail" but it does not kill the process.

https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Build.Tasks/StaticGraphRestoreTaskBase.cs#L211-L217

The try...catch which handles cancellation should be moved to surround just the Semaphore.Wait() since that's the only call that would throw an OperationCanceledException. The proceeding code would then kill the process and return true in the case where the user canceled the build.

Verbose Logs

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:RestoreStaticGraph Issues related to the Static Graph restore Priority:2 Issues for the current backlog. Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants