Refactored timers to wrap the object in a struct for improper use det…

…ection.

Moved timers and factories to `Proto.Timers` namespace.

Package/Core/InternalShared/PoolInternal.cs

@@ -142,7 +142,7 @@ static partial void MarkNotInPoolPrivate(object obj)
         } // class ObjectPool
 
         // LocalObjectPool is used for pooling on a per-instance basis, used with types that we don't have compile-time access to
-        // (e.g. pooled types associated with a public abstract class, like PoolableTimerFactory).
+        // (e.g. pooled types associated with a public abstract class, like TimerFactory).
 #if !PROTO_PROMISE_DEVELOPER_MODE
         [DebuggerNonUserCode, StackTraceHidden]
 #endif

Package/Core/Promises/Config.cs

@@ -7,6 +7,7 @@
 
 #pragma warning disable RECS0029 // Warns about property or indexer setters and event adders or removers that do not use the value parameter
 
+using Proto.Timers;
 using System;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
@@ -164,9 +165,9 @@ private static void ThrowCannotDisableAsyncFlow()
                 => throw new InvalidOperationException("Cannot disable AsyncFlowExecutionContext. It may only be enabled.");
 
             /// <summary>
-            /// The default <see cref="PoolableTimerFactory"/> to use for time-based methods when one is not provided.
+            /// The default <see cref="TimerFactory"/> to use for time-based methods when one is not provided.
             /// </summary>
-            public static PoolableTimerFactory DefaultTimerFactory
+            public static TimerFactory DefaultTimerFactory
             {
                 [MethodImpl(Internal.InlineOption)]
                 get => s_defaultTimeProvider;
@@ -175,16 +176,16 @@ public static PoolableTimerFactory DefaultTimerFactory
                 {
                     if (value == null)
                     {
-                        ThrowNullPoolableTimerFactory();
+                        ThrowNullTimerFactory();
                     }
                     s_defaultTimeProvider = value;
                 }
             }
-            private static PoolableTimerFactory s_defaultTimeProvider = PoolableTimerFactory.System;
+            private static TimerFactory s_defaultTimeProvider = TimerFactory.System;
 
             [MethodImpl(MethodImplOptions.NoInlining)]
-            private static void ThrowNullPoolableTimerFactory()
-                => throw new ArgumentNullException("value", $"{nameof(PoolableTimerFactory)} may not be null.");
+            private static void ThrowNullTimerFactory()
+                => throw new ArgumentNullException("value", $"{nameof(TimerFactory)} may not be null.");
         }
     }
 }

Package/Core/Threading/IPoolableTimer.cs → Package/Core/Timers/ITimerSource.cs

@@ -1,13 +1,11 @@
+using Proto.Promises;
 using System;
 using System.Threading;
 
-namespace Proto.Promises.Threading
+namespace Proto.Timers
 {
-    /// <summary>Represents a timer that can have its due time and period changed, and which may be pooled when it is disposed.</summary>
-    /// <remarks>
-    /// Users of this interface must ensure that <see cref="Change"/> is not called after <see cref="DisposeAsync"/>, and <see cref="DisposeAsync"/> is not called more than once.
-    /// </remarks>
-    public interface IPoolableTimer
+    /// <summary>Represents an object that can be wrapped by a <see cref="Timer"/>.</summary>
+    public interface ITimerSource
     {
         /// <summary>Changes the start time and the interval between method invocations for a timer, using <see cref="TimeSpan"/> values to measure time intervals.</summary>
         /// <param name="dueTime">
@@ -18,20 +16,18 @@ public interface IPoolableTimer
         /// The time interval between invocations of the callback method specified when the Timer was constructed.
         /// Specify <see cref="Timeout.InfiniteTimeSpan"/> to disable periodic signaling.
         /// </param>
+        /// <param name="token">An opaque value that was provided to the <see cref="Timer"/> constructor.</param>
         /// <returns><see langword="true"/> if the timer was successfully updated; otherwise, <see langword="false"/>.</returns>
         /// <exception cref="ArgumentOutOfRangeException">The <paramref name="dueTime"/> or <paramref name="period"/> parameter, in milliseconds, is less than -1 or greater than 4294967294.</exception>
-        /// <remarks>
-        /// This must not be called after <see cref="DisposeAsync"/>.
-        /// </remarks>
-        void Change(TimeSpan dueTime, TimeSpan period);
+        /// <exception cref="ObjectDisposedException">This was disposed.</exception>
+        void Change(TimeSpan dueTime, TimeSpan period, int token);
 
         /// <summary>
-        /// Releases the instance such that it may be returned to an object pool.
+        /// Releases resources used by the timer.
         /// </summary>
+        /// <param name="token">An opaque value that was provided to the <see cref="Timer"/> constructor.</param>
         /// <returns>A <see cref="Promise"/> that will be resolved when all timer callbacks have completed.</returns>
-        /// <remarks>
-        /// <see cref="Change(TimeSpan, TimeSpan)"/> must not be called after this is called, and this must not be called more than once.
-        /// </remarks>
-        Promise DisposeAsync();
+        /// <exception cref="ObjectDisposedException">This was already disposed.</exception>
+        Promise DisposeAsync(int token);
     }
 }

Package/Core/Utilities/Internal/PoolableSystemTimerFactoryInternal.cs → Package/Core/Timers/Internal/SystemTimerFactoryInternal.cs

@@ -4,29 +4,30 @@
 #undef PROMISE_DEBUG
 #endif
 
-using Proto.Promises.Threading;
+using Proto.Promises;
 using System;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Threading;
 
-namespace Proto.Promises
+namespace Proto.Timers
 {
-    partial class PoolableTimerFactory
+    partial class TimerFactory
     {
-        // Specialized version of PoolableTimeProviderTimerFactory for System.
+        // Specialized version of TimeProviderTimerFactory for System.
 #if !PROTO_PROMISE_DEVELOPER_MODE
         [DebuggerNonUserCode, StackTraceHidden]
 #endif
-        private class PoolableSystemTimerFactory : PoolableTimerFactory
+        private class SystemTimerFactory : TimerFactory
         {
-            public override IPoolableTimer CreateTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)
+            public override Timer CreateTimer(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)
             {
                 if (callback == null)
                 {
-                    throw new ArgumentNullException(nameof(callback), "callback may not be null", Internal.GetFormattedStacktrace(1));
+                    throw new Promises.ArgumentNullException(nameof(callback), "callback may not be null", Internal.GetFormattedStacktrace(1));
                 }
-                return PoolableSystemTimerFactoryTimer.GetOrCreate(callback, state, dueTime, period);
+                var timer = SystemTimerFactoryTimer.GetOrCreate(callback, state, dueTime, period);
+                return new Timer(timer, timer.Version);
             }
 
             public override TimeProvider ToTimeProvider()
@@ -36,13 +37,17 @@ public override TimeProvider ToTimeProvider()
 #if !PROTO_PROMISE_DEVELOPER_MODE
         [DebuggerNonUserCode, StackTraceHidden]
 #endif
-        private sealed class PoolableSystemTimerFactoryTimer : Internal.HandleablePromiseBase, IPoolableTimer
+        private sealed class SystemTimerFactoryTimer : Internal.HandleablePromiseBase, ITimerSource
         {
             // Timer doubles as the sync lock.
             private readonly ITimer _timer;
             private CallbackInvoker _callbackInvoker;
+            // Start with 1 instead of 0 to reduce risk of false positives.
+            private int _version = 1;
 
-            private PoolableSystemTimerFactoryTimer()
+            internal int Version => _version;
+
+            private SystemTimerFactoryTimer()
             {
                 // We don't need the extra overhead of the system timer capturing the execution context.
                 // We capture it manually per usage of this instance.
@@ -52,26 +57,26 @@ private PoolableSystemTimerFactoryTimer()
                 }
             }
 
-            ~PoolableSystemTimerFactoryTimer()
+            ~SystemTimerFactoryTimer()
             {
                 if (_callbackInvoker != null)
                 {
                     Internal.Discard(_callbackInvoker); // Prevent the invoker's base finalizer from adding an extra exception.
-                    Internal.ReportRejection(new UnreleasedObjectException($"A poolable timer was garbage collected without being disposed. {this}"), _callbackInvoker);
+                    Internal.ReportRejection(new UnreleasedObjectException($"A timer's resources were garbage collected without being disposed. {this}"), _callbackInvoker);
                 }
             }
 
             [MethodImpl(Internal.InlineOption)]
-            private static PoolableSystemTimerFactoryTimer GetOrCreate()
+            private static SystemTimerFactoryTimer GetOrCreate()
             {
-                var obj = Internal.ObjectPool.TryTakeOrInvalid<PoolableSystemTimerFactoryTimer>();
+                var obj = Internal.ObjectPool.TryTakeOrInvalid<SystemTimerFactoryTimer>();
                 return obj == Internal.PromiseRefBase.InvalidAwaitSentinel.s_instance
-                    ? new PoolableSystemTimerFactoryTimer()
-                    : obj.UnsafeAs<PoolableSystemTimerFactoryTimer>();
+                    ? new SystemTimerFactoryTimer()
+                    : obj.UnsafeAs<SystemTimerFactoryTimer>();
             }
 
             [MethodImpl(Internal.InlineOption)]
-            internal static PoolableSystemTimerFactoryTimer GetOrCreate(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)
+            internal static SystemTimerFactoryTimer GetOrCreate(TimerCallback callback, object state, TimeSpan dueTime, TimeSpan period)
             {
                 var timer = GetOrCreate();
                 var callbackInvoker = CallbackInvoker.GetOrCreate(callback, state);
@@ -106,32 +111,33 @@ private void OnTimerCallback(object _)
                 }
             }
 
-            public void Change(TimeSpan dueTime, TimeSpan period)
+            public void Change(TimeSpan dueTime, TimeSpan period, int token)
             {
                 lock (_timer)
                 {
                     var callbackInvoker = _callbackInvoker;
-                    if (callbackInvoker is null)
+                    if (callbackInvoker is null | _version != token)
                     {
-                        throw new ObjectDisposedException(nameof(IPoolableTimer));
+                        throw new ObjectDisposedException(nameof(SystemTimerFactoryTimer));
                     }
                     callbackInvoker.PrepareChange(dueTime, period);
                 }
 
                 _timer.Change(dueTime, period);
             }
 
-            public Promise DisposeAsync()
+            public Promise DisposeAsync(int token)
             {
                 CallbackInvoker callbackInvoker;
                 lock (_timer)
                 {
                     callbackInvoker = _callbackInvoker;
-                    if (callbackInvoker is null)
+                    if (callbackInvoker is null | _version != token)
                     {
-                        throw new ObjectDisposedException(nameof(IPoolableTimer));
+                        throw new ObjectDisposedException(nameof(SystemTimerFactoryTimer));
                     }
                     _callbackInvoker = null;
+                    unchecked { ++_version; }
                     callbackInvoker.PrepareChange(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
                 }
 
@@ -276,6 +282,6 @@ internal Promise DisposeAsync()
                     return new Promise(this, Id);
                 }
             } // class CallbackInvoker
-        } // class PoolableSystemTimerFactoryTimer
-    } // class PoolableTimerFactory
-} // namespace Proto.Promises
+        } // class SystemTimerFactoryTimer
+    } // class TimerFactory
+} // namespace Proto.Timers