Merge pull request #482 from timcassell/fix-promisenew-racecond

Fix race condition with `Promise.New`
timcassell · Oct 27, 2024 · b7e0d78 · b7e0d78
2 parents 177ddfd + 4f20dba
commit b7e0d78
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 2 deletions.
diff --git a/Package/Core/Promises/Internal/DeferredInternal.cs b/Package/Core/Promises/Internal/DeferredInternal.cs
@@ -148,8 +148,14 @@ private DeferredNewPromise() { }
 
                 internal override void MaybeDispose()
                 {
-                    Dispose();
-                    ObjectPool.MaybeRepool(this);
+                    // It is theoretically possible for this to be completed from the callback, and then the callback throws,
+                    // causing this to attempt to complete again. The thread could be starved while another thread
+                    // re-uses this object from the pool. This interlocked operation protects against that.
+                    if (InterlockedAddWithUnsignedOverflowCheck(ref _disposeCounter, -1) == 0)
+                    {
+                        Dispose();
+                        ObjectPool.MaybeRepool(this);
+                    }
                 }
 
                 [MethodImpl(InlineOption)]
@@ -166,6 +172,7 @@ internal static DeferredNewPromise<TResult, TDelegate> GetOrCreate(TDelegate run
                 {
                     var promise = GetOrCreate();
                     promise.Reset();
+                    promise._disposeCounter = 2;
                     promise._runner = runner;
                     return promise;
                 }
@@ -253,6 +260,7 @@ private void Run()
                         }
                     }
                     ClearCurrentInvoker();
+                    MaybeDispose();
                 }
             }
         } // class PromiseRef

diff --git a/Package/Core/Promises/Internal/PromiseFieldsInternal.cs b/Package/Core/Promises/Internal/PromiseFieldsInternal.cs
@@ -174,6 +174,7 @@ partial class DeferredPromise<TResult> : DeferredPromiseBase<TResult>
             partial class DeferredNewPromise<TResult, TDelegate> : DeferredPromise<TResult>
                 where TDelegate : IDelegateNew<TResult>
             {
+                private int _disposeCounter;
                 private TDelegate _runner;
             }