From 4f20dbabb04c2c3e7fcb06f78af6c06fb0d14729 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 23 Oct 2024 05:34:15 -0400 Subject: [PATCH] Fixed an unlikely race condition with `Promise.New`. --- Package/Core/Promises/Internal/DeferredInternal.cs | 12 ++++++++++-- .../Core/Promises/Internal/PromiseFieldsInternal.cs | 1 + 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Package/Core/Promises/Internal/DeferredInternal.cs b/Package/Core/Promises/Internal/DeferredInternal.cs index 08e00581..3b60c10f 100644 --- 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 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 index 51b29ba4..811fb3f7 100644 --- a/Package/Core/Promises/Internal/PromiseFieldsInternal.cs +++ b/Package/Core/Promises/Internal/PromiseFieldsInternal.cs @@ -174,6 +174,7 @@ partial class DeferredPromise : DeferredPromiseBase partial class DeferredNewPromise : DeferredPromise where TDelegate : IDelegateNew { + private int _disposeCounter; private TDelegate _runner; }