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 recommended way to wait synchronously the completion of a ValueTask is to convert it to a Task, with the AsTask method:

Task<CallResult<string>> task = CreateSpotListenKeyAsync().AsTask();
var result = task.GetAwaiter().GetResult();

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

In your case, and based on the implementation of the CreateSpotListenKeyAsync method, this conversion most likely incurs zero overhead. Assuming that the ValueTask<CallResult<string>> is not already completed upon creation, most likely it wraps internally a Task<CallResult<string>>, so the conversion will just return the wrapped task. It seems unlikely that it wraps an implementation of the IValueTaskSource<T> interface, in which case the conversion would incur the cost of allocating a few lightweight objects.

In case you anticipate that the ValueTask<CallResult<string>> will be frequently completed upon creation, you could optimize your code like this:

var valueTask = CreateSpotListenKeyAsync();
CallResult<string> result;
if (valueTask.IsCompletedSuccessfully)
{
    result = valueTask.Result;
}
else
{
    result = valueTask.AsTask().GetAwaiter().GetResult();
}

The intention of above code is to avoid an internal call to the Task.FromResult method, which is allocating in general.

It is also possible to optimize the synchronous waiting of IValueTaskSource<T>-based value tasks, but it's not trivial, and the expected benefits are so minuscule that it's unlikely to worth the effort.

---

In case of a non-generic ValueTask, you could do what @nop suggested in a comment:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void GetResult(this in ValueTask valueTask)
{
    if (valueTask.IsCompletedSuccessfully)
    {
        valueTask.GetAwaiter().GetResult();
        return;
    }

    valueTask.AsTask().GetAwaiter().GetResult();
}

But it's not really needed. In case the ValueTask has completed successfully, the AsTask will never allocate, whatever is the underlying implementation of the ValueTask. In case it is backed by a IValueTaskSource, the AsTask will return the static singleton Task.CompletedTask. So doing simply valueTask.AsTask().GetAwaiter().GetResult(); is pretty much the same.