Skip to content

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
@github-actions github-actions bot locked and limited conversation to collaborators Apr 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants