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

Add and use ConfigureAwaitOptions #87067

Merged
merged 3 commits into from
Jun 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/libraries/Common/src/System/Net/Http/X509ResourceClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace System.Net.Http
{
internal static partial class X509ResourceClient
{
private static readonly Func<string, CancellationToken, bool, ValueTask<byte[]?>>? s_downloadBytes = CreateDownloadBytesFunc();
private static readonly Func<string, CancellationToken, bool, Task<byte[]?>>? s_downloadBytes = CreateDownloadBytesFunc();

static partial void ReportNoClient();
static partial void ReportNegativeTimeout();
Expand All @@ -24,18 +24,17 @@ internal static partial class X509ResourceClient

internal static byte[]? DownloadAsset(string uri, TimeSpan downloadTimeout)
{
ValueTask<byte[]?> task = DownloadAssetCore(uri, downloadTimeout, async: false);
Task<byte[]?> task = DownloadAssetCore(uri, downloadTimeout, async: false);
Debug.Assert(task.IsCompletedSuccessfully);
return task.Result;
}

internal static Task<byte[]?> DownloadAssetAsync(string uri, TimeSpan downloadTimeout)
{
ValueTask<byte[]?> task = DownloadAssetCore(uri, downloadTimeout, async: true);
return task.AsTask();
return DownloadAssetCore(uri, downloadTimeout, async: true);
}

private static async ValueTask<byte[]?> DownloadAssetCore(string uri, TimeSpan downloadTimeout, bool async)
private static async Task<byte[]?> DownloadAssetCore(string uri, TimeSpan downloadTimeout, bool async)
{
if (s_downloadBytes is null)
{
Expand All @@ -60,8 +59,12 @@ internal static partial class X509ResourceClient

try
{
ret = await s_downloadBytes(uri, cts?.Token ?? default, async).ConfigureAwait(false);
return ret;
Task<byte[]?> task = s_downloadBytes(uri, cts?.Token ?? default, async);
await ((Task)task).ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing);
if (task.IsCompletedSuccessfully)
{
return task.Result;
}
Comment on lines +62 to +67
Copy link
Contributor

Choose a reason for hiding this comment

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

Just trying to understand this more: isn't this semantically different than the previous code? Specifically: the previous code would catch the exception if the task failed, which caused it to be marked as observed. The new code instead would just never observe the exception if the task failed, as it's just returning the result from the task if it did complete successfully. Wouldn't this make it so that if the task fails, the exception wrapper would not get its finalizer suppressed, and that exception would end up being signaled from the TaskScheduler.UnobservedTaskException event, which wasn't the case before this change? 🤔

Is that just by design, or am I missing something that makes this not the case here?
Thank you! 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh I had missed that! Will definitely have to keep in mind that for cases where you want to await without throwing but also flow any exceptions to TaskScheduler.UnobservedTaskException (eg. we're doing this in the MVVM Toolkit), this new configurable task awaiter shouldn't be used then. Thank you for the additional info! 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

In what situation would you want that? From my perspective, the event is there for cases where the dev simply didn't pay attention. Any gesture that says "I know what I'm doing" supresses it. Just because someone accesses the Exception property or allows the exception to be thrown doesn't mean they've actually reacted to the error... they could have done _ = t.Exception; or catch {}. I see SuppressThrowing as a similar gesture.

Copy link
Contributor

@Sergio0694 Sergio0694 Jun 4, 2023

Choose a reason for hiding this comment

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

It's used under one of the configurable options for the [RelayCommand] generator (see docs here). Consider:

partial class MyViewModel
{
    [RelayCommand]
    private async Task DoStuffAsync()
    {
    }
}

This will cause the generated command to directly await the method. If it throws, it throws. This is the default behavior and matches the usual developer expectation. If you don't handle exceptions, your app goes down. In most cases, you'd just use this, manually add a try/catch, and do all the exception handling/logging yourself from there.

partial class MyViewModel
{
    [RelayCommand(FlowExceptionsToTaskScheduler = true)]
    private async Task DoStuffAsync()
    {
    }
}

This will instead await the method by not throwing exceptions, which will instead just flow to TaskScheduler.UnobservedTaskException, but without crashing the app. This can be useful in cases where you eg. have some centralized logging system for exceptions from your entire app, from a handler to that event, and where you don't particularly care about a given command to fail (or at least, you don't want that to crash your app).

It's just to give developers a bit more flexibility. We've had several feedbacks from people using/liking this option 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm surprised you want to use UnobservedTaskException for that. The infrastructure is obviously responsible for consuming the task in this case and should also be responsible for dealing with any failures and routing them appropriately. What do you do if the exception is thrown synchronously out of such a Task-returning method, or if it's not Task-returning at all, or if it's a ValueTask backed by an IValueTaskSource that won't trigger UnobservedTaskException? async void methods, for example, queue exceptions to be rethrown on the original SynchronizationContext so that the app model's unhandled exception event can treat those the same as anything else unhandled by an event handler.

Copy link
Contributor

@Sergio0694 Sergio0694 Jun 4, 2023

Choose a reason for hiding this comment

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

"What do you do if the exception is thrown synchronously out of such a Task-returning method"

The command is just wrapping your async method, so the only other scenario where invoking that command might throw is if you throw from an event handler subscribed to one of the properties of the command that send a notification. But in that case a crash is expected, as it's the same exact behavior you'd get if any other of your handlers (say, for some property change elsewhere) was to just throw while running. It'd be weird if that didn't crash the app.

"if it's not Task-returning at all, or if it's a ValueTask backed by an IValueTaskSource that won't trigger UnobservedTaskException?"

You can only use [RelayCommand] for methods that return a Task (or void). If you tried to use it over a method that returned something else (like a ValueTask), the generator would just issue a diagnostic producing an error.

"async void methods"

If a user had an async void method with [RelayCommand], that's just an error on their side, and if that method threw an exception there would be nothing we could do. But then again, this would be exactly the same behavior you'd get for any other event handler being async void in your app. One of the points of [RelayCommand] supporting Task-returning methods is specifically so that it can correctly handle exceptions, as well as offering other useful functionality on top (eg. concurrency control, running checks, etc.). This actually makes me thing we might want to introduce a new analyzer to warn on async void methods with [RelayCommand]... I've seen a few of these in the wild 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

My point about "async void" was simply that it unifies with the exception handling mechanism of the app model, e.g. whether or not this event handler is marked "async", the same behavior results:
image
I'm surprised you're trying to create a different mechanism. I'm obviously not your end user, but if I was, I'd expect unhandled exceptions to end up in the same place, raising Application.ThreadException in WinForms, raising Dispatcher.UnhandledException in WPF, etc. UnobservedTaskException was not introduced to be a general exception handling choke point; it was introduced as a back stop for places where you inadvertently neglected to wait on a task. Since you're raising this case as an example of where you'd need to be careful with SuppressThrowing, you're obviously awaiting the task, which means you're going out of your way to use UnobservedTaskException as such a general exception handling choke point.

Copy link
Contributor

Choose a reason for hiding this comment

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

"I'm surprised you're trying to create a different mechanism"

But we're not doing that. Let me clarify. The default mechanism, which is when you just have your Task-returning method for your command, will have the command await that task behind the scenes (in what is ultimately an async void handler, since it's invoked by the UI via ICommand.Execute(object)). If that method throws an exception that isn't handled, the exception will bubble up and result in the same as you're showing above. The main difference is that with your (wrapped) method being asynchronous, you get additional benefit on top, such as the command being able to also notify when an operation is running (which is very convenient to bind to eg. a loading ring, or to disable other commands), or to handle concurrency (eg. you can have the command automatically disable itself if it's already running). But with respect to exception handling, it's the same as what you're describing 🙂

The difference is only if you opt-in to FlowExceptionsToTaskScheduler, in which case yes the behavior is different. But that's just a separate option, disabled by default, which you can just enable in specific cases if you do want that and know what you're doing and what it will result in. In almost all cases, you would just leave it off.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm surprised you're trying to create a different mechanism

But we're not doing that. [...] The difference is only if you opt-in to FlowExceptionsToTaskScheduler, in which case yes the behavior is different

That's the "different mechanism" I'm referring to. We never intended TaskScheduler.UnobservedTaskException to be used in this manner, as something folks would opt-in to sending lots of exceptions to. If you're awaiting the task in your infra, and you don't want to allow the exception to bubble up to the app model's mechanism, I'd expect MVVM toolkit to have its own event handler folks can listen to for that use.

Anyway, thanks for the info.

}
catch { }
finally
Expand All @@ -74,7 +77,7 @@ internal static partial class X509ResourceClient
return null;
}

private static Func<string, CancellationToken, bool, ValueTask<byte[]?>>? CreateDownloadBytesFunc()
private static Func<string, CancellationToken, bool, Task<byte[]?>>? CreateDownloadBytesFunc()
{
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace System.Threading
/// <summary>
/// Helper for performing asynchronous I/O on Windows implemented as queueing a work item that performs synchronous I/O, complete with cancellation support.
/// </summary>
internal sealed class AsyncOverSyncWithIoCancellation : IThreadPoolWorkItem, ICriticalNotifyCompletion
internal sealed class AsyncOverSyncWithIoCancellation
{
/// <summary>A thread handle for the current OS thread.</summary>
/// <remarks>This is lazily-initialized for the current OS thread. We rely on finalization to clean up after it when the thread goes away.</remarks>
Expand All @@ -32,23 +32,9 @@ internal sealed class AsyncOverSyncWithIoCancellation : IThreadPoolWorkItem, ICr
/// the callback wasn't and will never be invoked. If it's non-null, its completion represents the completion of the asynchronous callback.
/// </summary>
private volatile Task? CallbackCompleted;
/// <summary>The <see cref="Action"/> continuation object handed to this instance when used as an awaiter to scheduler work to the thread pool.</summary>
private Action? _continuation;

// awaitable / awaiter implementation that enables this instance to be awaited in order to queue
// execution to the thread pool. This is purely a cost-saving measure in order to reuse this
// object we already need as the queued work item.
public AsyncOverSyncWithIoCancellation GetAwaiter() => this;
public bool IsCompleted => false;
public void GetResult() { }
public void OnCompleted(Action continuation) => throw new NotSupportedException();
public void UnsafeOnCompleted(Action continuation)
{
Debug.Assert(_continuation is null);
_continuation = continuation;
ThreadPool.UnsafeQueueUserWorkItem(this, preferLocal: true);
}
void IThreadPoolWorkItem.Execute() => _continuation!();

/// <summary>Prevent external instantiation.</summary>
private AsyncOverSyncWithIoCancellation() { }

/// <summary>Queues the invocation of <paramref name="action"/> to the thread pool.</summary>
/// <typeparam name="TState">The type of the state passed to <paramref name="action"/>.</typeparam>
Expand All @@ -65,27 +51,24 @@ public void UnsafeOnCompleted(Action continuation)
[AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder))]
public static async ValueTask InvokeAsync<TState>(Action<TState> action, TState state, CancellationToken cancellationToken)
{
// Create the work item state object. This is used to pass around state through various APIs,
// while also serving double duty as the work item used to queue the operation to the thread pool.
var workItem = new AsyncOverSyncWithIoCancellation();

// Queue the work to the thread pool. This is implemented as a custom awaiter that queues the
// awaiter itself to the thread pool.
await workItem;
// Queue the work to complete asynchronously.
await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

// Register for cancellation, perform the work, and clean up. Even though we're in an async method, awaits _must not_ be used inside
// the using block, or else the I/O cancellation could both not work and negatively interact with I/O on another thread. The func
// _must_ be invoked on the same thread that invoked RegisterCancellation, with no intervening work.
await using (workItem.RegisterCancellation(cancellationToken).ConfigureAwait(false))
SyncAsyncWorkItemRegistration reg = RegisterCancellation(cancellationToken);
try
{
try
{
action(state);
}
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested && oce.CancellationToken != cancellationToken)
{
throw CreateAppropriateCancellationException(cancellationToken, oce);
}
action(state);
}
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested && oce.CancellationToken != cancellationToken)
{
throw CreateAppropriateCancellationException(cancellationToken, oce);
}
finally
{
await reg.DisposeAsync().ConfigureAwait(false);
}
}

Expand All @@ -105,27 +88,24 @@ public static async ValueTask InvokeAsync<TState>(Action<TState> action, TState
[AsyncMethodBuilder(typeof(PoolingAsyncValueTaskMethodBuilder<>))]
public static async ValueTask<TResult> InvokeAsync<TState, TResult>(Func<TState, TResult> func, TState state, CancellationToken cancellationToken)
{
// Create the work item state object. This is used to pass around state through various APIs,
// while also serving double duty as the work item used to queue the operation to the thread pool.
var workItem = new AsyncOverSyncWithIoCancellation();

// Queue the work to the thread pool. This is implemented as a custom awaiter that queues the
// awaiter itself to the thread pool.
await workItem;
// Queue the work to complete asynchronously.
await Task.CompletedTask.ConfigureAwait(ConfigureAwaitOptions.ForceYielding);

// Register for cancellation, perform the work, and clean up. Even though we're in an async method, awaits _must not_ be used inside
// the using block, or else the I/O cancellation could both not work and negatively interact with I/O on another thread. The func
// _must_ be invoked on the same thread that invoked RegisterCancellation, with no intervening work.
await using (workItem.RegisterCancellation(cancellationToken).ConfigureAwait(false))
SyncAsyncWorkItemRegistration reg = RegisterCancellation(cancellationToken);
try
{
try
{
return func(state);
}
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested && oce.CancellationToken != cancellationToken)
{
throw CreateAppropriateCancellationException(cancellationToken, oce);
}
return func(state);
}
catch (OperationCanceledException oce) when (cancellationToken.IsCancellationRequested && oce.CancellationToken != cancellationToken)
{
throw CreateAppropriateCancellationException(cancellationToken, oce);
}
finally
{
await reg.DisposeAsync().ConfigureAwait(false);
}
}

Expand All @@ -145,7 +125,7 @@ private static OperationCanceledException CreateAppropriateCancellationException
return newOce;
}

/// <summary>The struct IDisposable returned from <see cref="RegisterCancellation"/> in order to clean up after the registration.</summary>
/// <summary>The struct IDisposable returned from RegisterCancellation in order to clean up after the registration.</summary>
private struct SyncAsyncWorkItemRegistration : IDisposable, IAsyncDisposable
{
public AsyncOverSyncWithIoCancellation WorkItem;
Expand Down Expand Up @@ -201,44 +181,44 @@ public async ValueTask DisposeAsync()

/// <summary>Registers for cancellation with the specified token.</summary>
/// <remarks>Upon cancellation being requested, the implementation will attempt to CancelSynchronousIo for the thread calling RegisterCancellation.</remarks>
private SyncAsyncWorkItemRegistration RegisterCancellation(CancellationToken cancellationToken)
{
// If the token can't be canceled, there's nothing to register.
if (!cancellationToken.CanBeCanceled)
{
return default;
}
private static SyncAsyncWorkItemRegistration RegisterCancellation(CancellationToken cancellationToken) =>
cancellationToken.CanBeCanceled ? RegisterCancellation(new AsyncOverSyncWithIoCancellation(), cancellationToken) :
default; // If the token can't be canceled, there's nothing to register.

/// <summary>Registers for cancellation with the specified token.</summary>
/// <remarks>Upon cancellation being requested, the implementation will attempt to CancelSynchronousIo for the thread calling RegisterCancellation.</remarks>
private static SyncAsyncWorkItemRegistration RegisterCancellation(AsyncOverSyncWithIoCancellation instance, CancellationToken cancellationToken)
{
// Get a handle for the current thread. This is stored and used to cancel the I/O on this thread
// in response to the cancellation token having cancellation requested. If the handle is invalid,
// which could happen if OpenThread fails, skip attempts at cancellation. The handle needs to be
// opened with THREAD_TERMINATE in order to be able to call CancelSynchronousIo.
ThreadHandle = t_currentThreadHandle;
if (ThreadHandle is null)
instance.ThreadHandle = t_currentThreadHandle;
if (instance.ThreadHandle is null)
{
ThreadHandle = Interop.Kernel32.OpenThread(Interop.Kernel32.THREAD_TERMINATE, bInheritHandle: false, Interop.Kernel32.GetCurrentThreadId());
if (ThreadHandle.IsInvalid)
instance.ThreadHandle = Interop.Kernel32.OpenThread(Interop.Kernel32.THREAD_TERMINATE, bInheritHandle: false, Interop.Kernel32.GetCurrentThreadId());
if (instance.ThreadHandle.IsInvalid)
{
int lastError = Marshal.GetLastPInvokeError();
Debug.Fail($"{nameof(Interop.Kernel32.OpenThread)} unexpectedly failed with 0x{lastError:X8}: {Marshal.GetPInvokeErrorMessage(lastError)}");
return default;
}

t_currentThreadHandle = ThreadHandle;
t_currentThreadHandle = instance.ThreadHandle;
}

// Register with the token.
SyncAsyncWorkItemRegistration reg = default;
reg.WorkItem = this;
reg.WorkItem = instance;
reg.CancellationRegistration = cancellationToken.UnsafeRegister(static s =>
{
var state = (AsyncOverSyncWithIoCancellation)s!;
var instance = (AsyncOverSyncWithIoCancellation)s!;

// If cancellation was already requested when UnsafeRegister was called, it'll invoke
// the callback immediately. If we allowed that to loop until cancellation was successful,
// we'd deadlock, as we'd never perform the very I/O it was waiting for. As such, if
// the callback is invoked prior to be ready for it, we ignore the callback.
if (!state.FinishedCancellationRegistration)
if (!instance.FinishedCancellationRegistration)
{
return;
}
Expand All @@ -250,17 +230,17 @@ private SyncAsyncWorkItemRegistration RegisterCancellation(CancellationToken can
// this looping synchronously, we instead queue the invocation of the looping so that it
// runs asynchronously from the Cancel call. Then in order to be able to track its completion,
// we store the Task representing that asynchronous work, such that cleanup can wait for the Task.
state.CallbackCompleted = Task.Factory.StartNew(static s =>
instance.CallbackCompleted = Task.Factory.StartNew(static s =>
{
var state = (AsyncOverSyncWithIoCancellation)s!;
var instance = (AsyncOverSyncWithIoCancellation)s!;

// Cancel the I/O. If the cancellation happens too early and we haven't yet initiated
// the synchronous operation, CancelSynchronousIo will fail with ERROR_NOT_FOUND, and
// we'll loop to try again.
SpinWait sw = default;
while (state.ContinueTryingToCancel)
while (instance.ContinueTryingToCancel)
{
if (Interop.Kernel32.CancelSynchronousIo(state.ThreadHandle!))
if (Interop.Kernel32.CancelSynchronousIo(instance.ThreadHandle!))
{
// Successfully canceled I/O.
break;
Expand All @@ -276,12 +256,12 @@ private SyncAsyncWorkItemRegistration RegisterCancellation(CancellationToken can

sw.SpinOnce();
}
}, s, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}, this);
}, instance, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
}, instance);

// Now that we've registered with the token, tell the callback it's safe to enter
// its cancellation loop if the callback is invoked.
FinishedCancellationRegistration = true;
instance.FinishedCancellationRegistration = true;

// And now since cancellation may have been requested and we may have suppressed it
// until the previous line, check to see if cancellation has now been requested, and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,15 +603,6 @@ protected sealed class CustomTaskScheduler : TaskScheduler
protected override IEnumerable<Task> GetScheduledTasks() => new Task[0];
}

protected readonly struct JumpToThreadPoolAwaiter : ICriticalNotifyCompletion
{
public JumpToThreadPoolAwaiter GetAwaiter() => this;
public bool IsCompleted => false;
public void OnCompleted(Action continuation) => ThreadPool.QueueUserWorkItem(_ => continuation());
public void UnsafeOnCompleted(Action continuation) => ThreadPool.UnsafeQueueUserWorkItem(_ => continuation(), null);
public void GetResult() { }
}

protected sealed unsafe class NativeMemoryManager : MemoryManager<byte>
{
private readonly int _length;
Expand Down Expand Up @@ -2046,9 +2037,7 @@ public static IEnumerable<object[]> ReadAsync_ContinuesOnCurrentContextIfDesired
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "iOS/tvOS blocks binding to UNIX sockets")]
public virtual async Task ReadAsync_ContinuesOnCurrentSynchronizationContextIfDesired(bool flowExecutionContext, bool? continueOnCapturedContext)
{
await default(JumpToThreadPoolAwaiter); // escape xunit sync ctx

using StreamPair streams = await CreateConnectedStreamsAsync();
using StreamPair streams = await CreateConnectedStreamsAsync().ConfigureAwait(ConfigureAwaitOptions.ForceYielding /* escape xunit sync ctx */);
foreach ((Stream writeable, Stream readable) in GetReadWritePairs(streams))
{
Assert.Null(SynchronizationContext.Current);
Expand Down Expand Up @@ -2130,9 +2119,7 @@ public virtual async Task ReadAsync_ContinuesOnCurrentSynchronizationContextIfDe
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "iOS/tvOS blocks binding to UNIX sockets")]
public virtual async Task ReadAsync_ContinuesOnCurrentTaskSchedulerIfDesired(bool flowExecutionContext, bool? continueOnCapturedContext)
{
await default(JumpToThreadPoolAwaiter); // escape xunit sync ctx

using StreamPair streams = await CreateConnectedStreamsAsync();
using StreamPair streams = await CreateConnectedStreamsAsync().ConfigureAwait(ConfigureAwaitOptions.ForceYielding /* escape xunit sync ctx */);
foreach ((Stream writeable, Stream readable) in GetReadWritePairs(streams))
{
Assert.Null(SynchronizationContext.Current);
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/tests/System/TimeProviderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ public static void RunDelayTests(TimeProvider provider, ITestTaskFactory taskFac

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
[MemberData(nameof(TimersProvidersWithTaskFactorData))]
public static async void RunWaitAsyncTests(TimeProvider provider, ITestTaskFactory taskFactory)
public static async Task RunWaitAsyncTests(TimeProvider provider, ITestTaskFactory taskFactory)
{
CancellationTokenSource cts = new CancellationTokenSource();

Expand Down Expand Up @@ -377,7 +377,7 @@ public static async void RunWaitAsyncTests(TimeProvider provider, ITestTaskFacto
#if !NETFRAMEWORK
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
[MemberData(nameof(TimersProvidersListData))]
public static async void PeriodicTimerTests(TimeProvider provider)
public static async Task PeriodicTimerTests(TimeProvider provider)
{
var timer = new PeriodicTimer(TimeSpan.FromMilliseconds(1), provider);
Assert.True(await timer.WaitForNextTickAsync());
Expand Down
Loading