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 a potential NullReferenceException in PlacementWorker #9386

Merged
merged 2 commits into from
Mar 16, 2025

Conversation

ledjon-behluli
Copy link
Contributor

@ledjon-behluli ledjon-behluli commented Mar 16, 2025

Fixes a potential issue where Task.Exception could be null due to race conditions or when tasks were canceled but not explicitly caught by the user. Instead of relying on implicit cancellation handling, this change ensures that exceptions are properly set when needed, preventing unexpected null reference issues in downstream logic.

Original thread: https://discord.com/channels/333727978460676096/922945034427432980/threads/1346762281161199668

Microsoft Reviewers: Open in CodeFlow

Comment on lines 379 to 385
if (exception is null)
{
// Due to race conditions, it is possible to observe IsFaulted = true, but still Exception = null.
// This is because the state transition to Faulted might have occurred just after the Exception
// property check, but before the internal exception was retrieved.

return new Exception("Task faulted without an exception.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure about this? I don't believe it's true. If it were, that would be a serious bug in the .NET runtime.

Copy link
Contributor Author

@ledjon-behluli ledjon-behluli Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, it is a trade off to avoid locking, see: https://source.dot.net/System.Private.CoreLib/R/0bdc783f2cd45895.html

@ReubenBond
Copy link
Member

LGTM, thanks @ledjon-behluli!

@ReubenBond ReubenBond changed the title Fixes a potential NRE on PlacementWorker Fix a potential NullReferenceException in PlacementWorker Mar 16, 2025
@ReubenBond ReubenBond merged commit d64ed82 into dotnet:main Mar 16, 2025
24 of 25 checks passed
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.

2 participants