Address potential races in recent event changes (#882)

* Change ordering of delegate registration. * Fix potential race. * Address PR feedback.
microsoft · Jun 17, 2021 · d607f3e · d607f3e
1 parent ea43282
commit d607f3e
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 9 deletions.
diff --git a/src/cswinrt/code_writers.h b/src/cswinrt/code_writers.h
@@ -6488,11 +6488,12 @@ bind<write_type_name>(type, typedef_name_type::CCW, true)
                 {
                     handler = (%) =>
                     {
-                        if (_state.del == null)
+                        var localDel = _state.del;
+                        if (localDel == null)
                         {
                             return %;
                         }
-                        %_state.del.Invoke(%);
+                        %localDel.Invoke(%);
                     };
                 }
                 return handler;

diff --git a/src/cswinrt/strings/WinRT.cs b/src/cswinrt/strings/WinRT.cs
@@ -370,7 +370,10 @@ public void Subscribe(TDelegate del)
         {
             lock (this)
             {
-                if (_state.del is null)
+                bool registerHandler = _state.del is null;
+
+                _state.del = (TDelegate)global::System.Delegate.Combine(_state.del, del);
+                if (registerHandler)
                 {
                     var marshaler = CreateMarshaler((TDelegate)EventInvoke);
                     try
@@ -385,7 +388,6 @@ public void Subscribe(TDelegate del)
                         DisposeMarshaler(marshaler);
                     }
                 }
-                _state.del = (TDelegate)global::System.Delegate.Combine(_state.del, del);
             }
         }
 
@@ -445,10 +447,10 @@ private class Cache
             }
 
             private IWeakReference target;
-            private ConcurrentDictionary<int, EventSource<TDelegate>.State> states = new ConcurrentDictionary<int, EventSource<TDelegate>.State>();
+            private readonly ConcurrentDictionary<int, EventSource<TDelegate>.State> states = new ConcurrentDictionary<int, EventSource<TDelegate>.State>();
 
-            private static ReaderWriterLockSlim cachesLock = new ReaderWriterLockSlim();
-            private static ConcurrentDictionary<IntPtr, Cache> caches = new ConcurrentDictionary<IntPtr, Cache>();
+            private static readonly ReaderWriterLockSlim cachesLock = new ReaderWriterLockSlim();
+            private static readonly ConcurrentDictionary<IntPtr, Cache> caches = new ConcurrentDictionary<IntPtr, Cache>();
 
             private Cache Update(IWeakReference target, EventSource<TDelegate> source, int index)
             {
@@ -579,8 +581,9 @@ override protected System.Delegate EventInvoke
                 {
                     handler = (System.Object obj, T e) =>
                     {
-                        if (_state.del != null)
-                            _state.del.Invoke(obj, e);
+                        var localDel = _state.del;
+                        if (localDel != null)
+                            localDel.Invoke(obj, e);
                     };
                 }
                 return handler;