ValueTask instances should not have their result directly accessed unless the instance has already completed

Question

There is a library returning a ValueTask and I have a synchronous method which is consuming the ValueTask. The issue is that there is the following warning:

CA2012: ValueTask instances should not have their result directly accessed unless the instance has already completed. Unlike Tasks, calling Result or GetAwaiter().GetResult() on a ValueTask is not guaranteed to block until the operation completes. If you can't simply await the instance, consider first checking its IsCompleted property (or asserting it's true if you know that to be the case).

How do I fix it?

public void CreateListenKey()
{
    var result = CreateSpotListenKeyAsync().GetAwaiter().GetResult(); // CA2012: ValueTask instances should not have their result directly accessed unless the instance has already completed. Unlike Tasks, calling Result or GetAwaiter().GetResult() on a ValueTask is not guaranteed to block until the operation completes. If you can't simply await the instance, consider first checking its IsCompleted property (or asserting it's true if you know that to be the case).

    if (result.Success)
    {
        using var document = JsonDocument.Parse(result.Data!);
        lock (_listenKeyLocker)
        {
            if (document.RootElement.TryGetProperty("listenKey", out var listenKeyElement))
            {
                var listenKey = listenKeyElement.GetString();
                ListenKey = listenKey;
            }
        }
    }
}

// library
public async ValueTask<CallResult<string>> CreateSpotListenKeyAsync()
{
    var result = await SendPublicAsync<string>(
        "/api/v3/userDataStream",
        Method.Post);

    return result;
}

// Can't just make it async, because these listen key methods are used in an event handler.
private void OnKeepAliveTimerElapsed(object? sender, ElapsedEventArgs e)
{
    RestApi.PingListenKey();
}

Answer 1

The best way to wait synchronously for the completion of a ValueTask<T> is to Preserve it first:

var result = CreateSpotListenKeyAsync().Preserve().GetAwaiter().GetResult();

if (result.Success)
{
    //...

The Preserve method converts the ValueTask<T> into an instance backed by a regular Task<T>, which is safe to wait synchronously for its completion. In other words it ensures that the .GetAwaiter().GetResult() will block the current thread until it completes. Without the Preserve, the .GetAwaiter().GetResult() is not guaranteed to block. Value tasks that are backed by IValueTaskSource<T>s typically don't block on the .GetAwaiter().GetResult().

The Preserve method is similar to AsTask, but it returns the same ValueTask<T> instance when the value task represents a successful synchronously completed operation. The advantage of using the Preserve over the AsTask, is that it avoids the allocation of a Task<T> object in case the value task is already completed successfully. In other words it avoids an internal call to the Task.FromResult method, which is allocating in general.

In case of a non-generic ValueTask (without <TResult>), the advantage of the Preserve over the AsTask evaporates. Both the .Preserve().GetAwaiter().GetResult(); and the .AsTask().GetAwaiter().GetResult(); behave exactly the same, and are equally efficient. In case the value task is already completed successfully, the .AsTask() returns the static singleton Task.CompletedTask, so no allocation occurs.

_{This answer was rewritten from scratch. The original version advocated the use of the AsTask method.}