Merge pull request #880 from akarnokd/SystemClockChangeCrashFix

4.x: Fix InvalidOp when enumerating the SystemClockChanged hashset

Author: Oren Novotny

Rx.NET/Source/src/System.Reactive/Concurrency/LocalScheduler.TimerQueue.cs

@@ -370,7 +370,7 @@ private static void EvaluateLongTermQueue()
         /// </summary>
         /// <param name="args">Currently not used.</param>
         /// <param name="sender">Currently not used.</param>
-        internal void SystemClockChanged(object sender, SystemClockChangedEventArgs args)
+        internal virtual void SystemClockChanged(object sender, SystemClockChangedEventArgs args)
         {
             lock (StaticGate)
             {

Rx.NET/Source/src/System.Reactive/Internal/SystemClock.cs

@@ -21,7 +21,7 @@ public static class SystemClock
     {
         private static readonly Lazy<ISystemClock> ServiceSystemClock = new Lazy<ISystemClock>(InitializeSystemClock);
         private static readonly Lazy<INotifySystemClockChanged> ServiceSystemClockChanged = new Lazy<INotifySystemClockChanged>(InitializeSystemClockChanged);
-        private static readonly HashSet<WeakReference<LocalScheduler>> SystemClockChanged = new HashSet<WeakReference<LocalScheduler>>();
+        internal static readonly HashSet<WeakReference<LocalScheduler>> SystemClockChanged = new HashSet<WeakReference<LocalScheduler>>();
         private static IDisposable _systemClockChangedHandlerCollector;
 
         private static int _refCount;
@@ -55,11 +55,13 @@ public static void Release()
             }
         }
 
-        private static void OnSystemClockChanged(object sender, SystemClockChangedEventArgs e)
+        internal static void OnSystemClockChanged(object sender, SystemClockChangedEventArgs e)
         {
             lock (SystemClockChanged)
             {
-                foreach (var entry in SystemClockChanged)
+                // create a defensive copy as the callbacks may change the hashset
+                var copySystemClockChanged = new List<WeakReference<LocalScheduler>>(SystemClockChanged);
+                foreach (var entry in copySystemClockChanged)
                 {
                     if (entry.TryGetTarget(out var scheduler))
                     {

Rx.NET/Source/tests/Tests.System.Reactive/Tests/SystemClockTest.cs

@@ -905,6 +905,37 @@ private static void ClockChanged_RefCounting_Callback()
             Assert.Equal(0, scm.N);
         }
 
+        [Fact]
+        public void SystemClockChange_SignalNoInvalidOperationExceptionDueToRemove()
+        {
+            var local = new RemoveScheduler();
+            SystemClock.SystemClockChanged.Add(new WeakReference<LocalScheduler>(local));
+
+            SystemClock.OnSystemClockChanged(this, new SystemClockChangedEventArgs());
+        }
+
+        private class RemoveScheduler : LocalScheduler
+        {
+            public override IDisposable Schedule<TState>(TState state, TimeSpan dueTime, Func<IScheduler, TState, IDisposable> action)
+            {
+                throw new NotImplementedException();
+            }
+
+            internal override void SystemClockChanged(object sender, SystemClockChangedEventArgs args)
+            {
+                var target = default(WeakReference<LocalScheduler>);
+                foreach (var entries in SystemClock.SystemClockChanged)
+                {
+                    if (entries.TryGetTarget(out var local) && local == this)
+                    {
+                        target = entries;
+                        break;
+                    }
+                }
+                SystemClock.SystemClockChanged.Remove(target);
+            }
+        }
+
         private class MyScheduler : LocalScheduler
         {
             internal List<ScheduledItem<TimeSpan>> _queue = new List<ScheduledItem<TimeSpan>>();