From 91101229193d48715b30e2c050ebec55b6944c62 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 26 Oct 2018 22:19:45 -0700 Subject: [PATCH] Add IAsyncDisposable implementation to Timer (dotnet/coreclr#20646) Commit migrated from https://github.com/dotnet/coreclr/commit/46dbd2796bfd28c56a9fb9b2f37c068225595f66 --- .../System.Private.CoreLib/Resources/Strings.resx | 3 + .../src/System/Threading/Timer.cs | 93 ++++++++++++++++++++-- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx b/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx index 26f5b66..de7eb5ba 100644 --- a/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx +++ b/src/coreclr/src/System.Private.CoreLib/Resources/Strings.resx @@ -2620,6 +2620,9 @@ Timeouts are not supported on this stream. + + The Timer was already closed using an incompatible Dispose method. + The given type cannot be boxed. diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Timer.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Timer.cs index efea457..8eb2310 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Timer.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Timer.cs @@ -9,6 +9,7 @@ using System.Diagnostics; using System.Diagnostics.Tracing; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using System.Threading.Tasks; namespace System.Threading { @@ -523,10 +524,11 @@ namespace System.Threading // after all pending callbacks are complete. We set m_canceled to prevent any callbacks that // are already queued from running. We track the number of callbacks currently executing in // m_callbacksRunning. We set m_notifyWhenNoCallbacksRunning only when m_callbacksRunning - // reaches zero. + // reaches zero. Same applies if Timer.DisposeAsync() is used, except with a Task + // instead of with a provided WaitHandle. private int m_callbacksRunning; private volatile bool m_canceled; - private volatile WaitHandle m_notifyWhenNoCallbacksRunning; + private volatile object m_notifyWhenNoCallbacksRunning; // may be either WaitHandle or Task internal TimerQueueTimer(TimerCallback timerCallback, object state, uint dueTime, uint period, bool flowExecutionContext) @@ -605,10 +607,7 @@ namespace System.Threading m_canceled = true; m_notifyWhenNoCallbacksRunning = toSignal; m_associatedTimerQueue.DeleteTimer(this); - - if (m_callbacksRunning == 0) - shouldSignal = true; - + shouldSignal = m_callbacksRunning == 0; success = true; } } @@ -619,6 +618,62 @@ namespace System.Threading return success; } + public ValueTask CloseAsync() + { + lock (m_associatedTimerQueue) + { + object notifyWhenNoCallbacksRunning = m_notifyWhenNoCallbacksRunning; + + // Mark the timer as canceled if it's not already. + if (m_canceled) + { + if (notifyWhenNoCallbacksRunning is WaitHandle) + { + // A previous call to Close(WaitHandle) stored a WaitHandle. We could try to deal with + // this case by using ThreadPool.RegisterWaitForSingleObject to create a Task that'll + // complete when the WaitHandle is set, but since arbitrary WaitHandle's can be supplied + // by the caller, it could be for an auto-reset event or similar where that caller's + // WaitOne on the WaitHandle could prevent this wrapper Task from completing. We could also + // change the implementation to support storing multiple objects, but that's not pay-for-play, + // and the existing Close(WaitHandle) already discounts this as being invalid, instead just + // returning false if you use it multiple times. Since first calling Timer.Dispose(WaitHandle) + // and then calling Timer.DisposeAsync is not something anyone is likely to or should do, we + // simplify by just failing in that case. + return new ValueTask(Task.FromException(new InvalidOperationException(SR.InvalidOperation_TimerAlreadyClosed))); + } + } + else + { + m_canceled = true; + m_associatedTimerQueue.DeleteTimer(this); + } + + // We've deleted the timer, so if there are no callbacks queued or running, + // we're done and return an already-completed value task. + if (m_callbacksRunning == 0) + { + return default; + } + + Debug.Assert( + notifyWhenNoCallbacksRunning == null || + notifyWhenNoCallbacksRunning is Task); + + // There are callbacks queued or running, so we need to store a Task + // that'll be used to signal the caller when all callbacks complete. Do so as long as + // there wasn't a previous CloseAsync call that did. + if (notifyWhenNoCallbacksRunning == null) + { + var t = new Task((object)null, TaskCreationOptions.RunContinuationsAsynchronously); + m_notifyWhenNoCallbacksRunning = t; + return new ValueTask(t); + } + + // A previous CloseAsync call already hooked up a task. Just return it. + return new ValueTask((Task)notifyWhenNoCallbacksRunning); + } + } + void IThreadPoolWorkItem.Execute() => Fire(); internal void Fire() @@ -651,7 +706,17 @@ namespace System.Threading internal void SignalNoCallbacksRunning() { - Interop.Kernel32.SetEvent(m_notifyWhenNoCallbacksRunning.SafeWaitHandle); + object toSignal = m_notifyWhenNoCallbacksRunning; + Debug.Assert(toSignal is WaitHandle || toSignal is Task); + + if (toSignal is WaitHandle wh) + { + Interop.Kernel32.SetEvent(wh.SafeWaitHandle); + } + else + { + ((Task)toSignal).TrySetResult(true); + } } internal void CallCallback() @@ -723,10 +788,17 @@ namespace System.Threading GC.SuppressFinalize(this); return result; } + + public ValueTask CloseAsync() + { + ValueTask result = m_timer.CloseAsync(); + GC.SuppressFinalize(this); + return result; + } } - public sealed class Timer : MarshalByRefObject, IDisposable + public sealed class Timer : MarshalByRefObject, IDisposable, IAsyncDisposable { private const uint MAX_SUPPORTED_TIMEOUT = (uint)0xfffffffe; @@ -868,5 +940,10 @@ namespace System.Threading { m_timer.Close(); } + + public ValueTask DisposeAsync() + { + return m_timer.CloseAsync(); + } } } -- 2.7.4