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

VSTHRD002 no longer warns on Task.Result/Wait/GetResult/etc after Task.WhenAll (in cases where proved to be safe) #878

Merged
merged 1 commit into from Aug 17, 2021

Conversation

bluetarpmedia
Copy link
Contributor

@bluetarpmedia bluetarpmedia commented Jul 12, 2021

Closes #693 (I think there's a typo in which it refers to VSTHRD103 but the warning comes from VSTHRD002)

VSTHRD002 now checks if the task has completed via Task.WhenAll prior to the problematic member access (e.g. Result, Wait, GetResult), and that the task variable has not been used in a way that would invalidate this test.

No longer produces VSTHRD002 warnings

var task1 = Task.Run(() => 1);
var task2 = Task.Run(() => 2);
await Task.WhenAll(task1, task2);
return task1.Result + task2.Result;  // No warnings
// Or GetAwaiter().GetResult()
var task1 = Task.Run(() => 1);
var task2 = Task.Run(() => 2);
var task3 = Task.Run(() => 3);
var task4 = Task.Run(() => 4);

await Task.WhenAll(task1, task2);
int val = task1.Result + task2.Result;  // No warnings

// Use of task3 / task4 would warn here

await Task.WhenAll(task3, task4);
val += task3.Result + task4.Result;  // No warnings
void PassTaskByValue(Task<int> task)
{
}

void Foo()
{
    int value = jtf.Run(async delegate
    {
        var task1 = Task.Run(() => 1);
        var task2 = Task.Run(() => 2);
        await Task.WhenAll(task1, task2);
        PassTaskByValue(task1);  // This is fine, task1 is still completed
        return task1.Result;     // No warning
    });
}

Correctly produces VSTHRD002 warnings because task variable is modified after WhenAll

var task1 = Task.Run(() => 1);
var task2 = Task.Run(() => 2);
await Task.WhenAll(task1, task2);
int val = task1.Result;        // No warning here because of preceding WhenAll

task1 = Task.Run(() => 11);
return val + task1.Result;     // Warning here; `task1` is now a different task
void PassTaskByRef(ref Task<int> task)  // Or `out` param
{
    task = Task.Run(() => 3);
}

void Foo()
{
    int value = jtf.Run(async delegate
    {
        var task1 = Task.Run(() => 1);
        var task2 = Task.Run(() => 2);
        await Task.WhenAll(task1, task2);
        PassTaskByRef(ref task1);
        return task1.Result;     // Warning here; assume the by-ref has modified the variable
    });
}

Still (incorrectly) produces VSTHRD002 warnings
Cases like these are too difficult / impossible to prove with static analysis.

var task1 = Task.Run(() => 1);
var task2 = Task.Run(() => 2);

List<Task<int>> list;
list.Add(task1);
list.Add(task2);

await Task.WhenAll(list);
task1.Result;  // Still warns
var list = GetTasks();
await Task.WhenAll(list);
list[0].Result;  // Still warns

EDIT: On second thought, that last example might be possible.

…ia Task.WhenAll prior to the problematic member access (e.g. Result/Wait), and that the task variable has not been used in-between in a way that would invalidate this check.
Copy link
Member

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

🎉Thanks!

@AArnott AArnott added this to the v17.0 milestone Aug 17, 2021
@AArnott AArnott merged commit 5f754ee into microsoft:main Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.Result after Task.WhenAll raise VSTHRD002
2 participants