Valuetask Consumed Once

ID

csharp.valuetask_consumed_once

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

async, code_smell, concurrency, race-condition

Description

Reports a ValueTask or ValueTask<T> local that is consumed twice: awaited again, or awaited and then read through .Result or .GetAwaiter(). A ValueTask may be consumed only once, and a second consumption reads an object the runtime is free to have recycled.

Rationale

A Task is an ordinary object that keeps its result for as long as anything references it, so awaiting the same one repeatedly is wasteful and nothing more. A ValueTask exists precisely to avoid allocating that object: when the operation does not complete synchronously it wraps a pooled source, and consuming the ValueTask is the act that returns that source to the pool.

So after the first consumption the wrapper refers to an object the pool may already have handed to an unrelated caller and reinitialised for a different operation. Reading it a second time is a race with whoever holds it now. The result may be the original value, or the value of an unrelated operation, or an exception, and which one you get depends on timing. That is why the documented contract is not "prefer to consume once" but "consume at most once, anything else is undefined".

The practical consequence is a defect that passes review and passes tests. Under no concurrency the pooled object is usually still untouched, so the second read returns the right answer; under production concurrency it starts returning another request’s data, and the symptom is wrong values rather than an error.

Task locals are not reported: nothing is recycled there. Locals declared with an inferred type are not reported either — the rule stays silent rather than claim recycling it cannot demonstrate. Two consumptions in mutually exclusive branches are not reported, because only one of them runs.

using System.Threading.Tasks;

public class Loader
{
    public async Task<int> Twice()
    {
        ValueTask<int> pending = LoadAsync();
        int first = await pending;
        int second = await pending;   // FLAW — the pooled source may already be reused
        return first + second;
    }

    public async Task<int> Once()
    {
        ValueTask<int> pending = LoadAsync();
        return await pending;   // OK, consumed exactly once
    }

    public async Task<int> Reusable()
    {
        Task<int> shared = LoadAsync().AsTask();
        return await shared + await shared;   // OK, AsTask gives a value that survives reuse
    }

    private ValueTask<int> LoadAsync() => new ValueTask<int>(7);
}

Remediation

Await once and keep what the await produced, then use that value wherever the second consumption was:

  • store the awaited result in a local and read the local again;

  • when the value genuinely has to be consumed more than once — passed to two consumers, awaited in a retry — call AsTask() and share the resulting task instead. That reintroduces the allocation the ValueTask avoided, which is the correct trade when the value has to be reusable;

  • when a method returns a ValueTask that its callers need to consume repeatedly, have it return a Task instead. ValueTask pays off on a hot path whose result is awaited immediately and once; it is the wrong return type for anything that is handed around.

Do not attempt to make the second read safe by checking IsCompleted first. That property is subject to the same single-consumption rule and reading it after consumption is equally undefined.

References