Handle combine

ProtoPromise/ProtoPromise.csproj

@@ -11,9 +11,6 @@
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <!--Set true to help debug internal promise code (allows the debugger to step into the code).-->
     <DeveloperMode>false</DeveloperMode>
-    <!--Set false to not unwind the stack before invoking the next continuation.
-        This can help to debug internal promise code with longer stack traces, but may result in a StackOverflowException if there are many continuations.-->
-    <AllowStackToUnwind>true</AllowStackToUnwind>
   </PropertyGroup>
 
   <ItemGroup>
@@ -26,10 +23,6 @@
     <DefineConstants>$(DefineConstants);TRACE;PROTO_PROMISE_DEVELOPER_MODE</DefineConstants>
   </PropertyGroup>
 
-  <PropertyGroup Condition="'$(AllowStackToUnwind)'=='false'">
-    <DefineConstants>$(DefineConstants);PROTO_PROMISE_NO_STACK_UNWIND</DefineConstants>
-  </PropertyGroup>
-
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release_NoProgress|AnyCPU'">
     <DefineConstants>$(DefineConstants);RELEASE;PROTO_PROMISE_PROGRESS_DISABLE</DefineConstants>
     <DebugSymbols>false</DebugSymbols>

ProtoPromiseTests/ProtoPromiseTests.csproj

@@ -3,7 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>net5.0;net472</TargetFrameworks>
     <IsPackable>false</IsPackable>
-    <DefineConstants>TRACE;CSHARP_7_3_OR_NEWER;CSHARP_7_OR_LATER</DefineConstants>
+    <DefineConstants>TRACE;CSHARP_7_3_OR_NEWER</DefineConstants>
     <Configurations>Release;Debug;Release_NoProgress;Debug_NoProgress</Configurations>
     <LangVersion>7.3</LangVersion>
   </PropertyGroup>
@@ -34,6 +34,7 @@
 
   <ItemGroup>
     <Compile Include="..\ProtoPromise_Unity\Assets\Plugins\ProtoPromiseTests\**/*.cs" />
+    <Compile Remove="..\ProtoPromise_Unity\Assets\Plugins\ProtoPromiseTests\Threading\ForOldRuntime\**/*" />
   </ItemGroup>
 
 </Project>

ProtoPromise_Unity/Assets/Plugins/ProtoPromise/Core/Cancelations/Internal/CancelationInternal.cs

@@ -99,11 +99,19 @@ private sealed class CancelableWrappe<TCancelable> : CancelableBase, ITraceable,
 
                 ~CancelableWrappe()
                 {
-                    if (!_disposed)
+                    try
                     {
-                        // For debugging. This should never happen.
-                        string message = "A " + GetType() + " was garbage collected without it being disposed.";
-                        AddRejectionToUnhandledStack(new UnreleasedObjectException(message), this);
+                        if (!_disposed)
+                        {
+                            // For debugging. This should never happen.
+                            string message = "A " + GetType() + " was garbage collected without it being disposed.";
+                            AddRejectionToUnhandledStack(new UnreleasedObjectException(message), this);
+                        }
+                    }
+                    catch (Exception e)
+                    {
+                        // This should never happen.
+                        AddRejectionToUnhandledStack(e, this);
                     }
                 }
 #endif
@@ -400,16 +408,24 @@ private enum State : int
 
             ~CancelationRef()
             {
-                if (_idsAndRetains._userRetains > 0)
+                try
                 {
-                    // CancelationToken wasn't released.
-                    string message = "A CancelationToken's resources were garbage collected without being released. You must release all IRetainable objects that you have retained.";
-                    AddRejectionToUnhandledStack(new UnreleasedObjectException(message), this);
+                    if (_idsAndRetains._userRetains > 0)
+                    {
+                        // CancelationToken wasn't released.
+                        string message = "A CancelationToken's resources were garbage collected without being released. You must release all IRetainable objects that you have retained.";
+                        AddRejectionToUnhandledStack(new UnreleasedObjectException(message), this);
+                    }
+                    if (_state != (int) State.Disposed)
+                    {
+                        // CancelationSource wasn't disposed.
+                        AddRejectionToUnhandledStack(new UnreleasedObjectException("CancelationSource's resources were garbage collected without being disposed."), this);
+                    }
                 }
-                if (_state != (int) State.Disposed)
+                catch (Exception e)
                 {
-                    // CancelationSource wasn't disposed.
-                    AddRejectionToUnhandledStack(new UnreleasedObjectException("CancelationSource's resources were garbage collected without being disposed."), this);
+                    // This should never happen.
+                    AddRejectionToUnhandledStack(e, this);
                 }
             }
 

ProtoPromise_Unity/Assets/Plugins/ProtoPromise/Core/InternalShared/HelperFunctionsInternal.cs

@@ -139,18 +139,6 @@ private ExecutionScheduler(SynchronizationHandler synchronizationHandler, bool i
 #endif
             }
 
-            [MethodImpl(InlineOption)]
-            internal ExecutionScheduler GetEmptyCopy()
-            {
-                bool isExecutingProgress =
-#if PROMISE_PROGRESS && (PROMISE_DEBUG || PROTO_PROMISE_DEVELOPER_MODE)
-                    _isExecutingProgress;
-#else
-                    false;
-#endif
-                return new ExecutionScheduler(_synchronizationHandler, isExecutingProgress);
-            }
-
             internal void Execute()
             {
                 // In case this is executed from a background thread, catch the exception and report it instead of crashing the app.
@@ -179,11 +167,7 @@ internal void Execute()
             internal void ScheduleSynchronous(HandleablePromiseBase handleable)
             {
                 AssertNotExecutingProgress();
-#if PROTO_PROMISE_NO_STACK_UNWIND // Helps to see full causality trace with internal stacktraces in exceptions (may cause StackOverflowException if the chain is very long).
-                handleable.Handle(ref this);
-#else
                 _handleStack.Push(handleable);
-#endif
             }
 
             internal void ScheduleOnContext(SynchronizationContext synchronizationContext, HandleablePromiseBase handleable)
@@ -221,7 +205,7 @@ private static void ExecuteFromContext(object state)
                 // In case this is executed from a background thread, catch the exception and report it instead of crashing the app.
                 try
                 {
-                    ExecutionScheduler executionScheduler = new ExecutionScheduler(true);
+                    var executionScheduler = new ExecutionScheduler(true);
                     ((HandleablePromiseBase) state).Handle(ref executionScheduler);
                     executionScheduler.Execute();
                 }
@@ -337,6 +321,9 @@ internal static bool TryGetValue<TValue>(this ValueContainer valueContainer, out
 
         internal static void AddUnhandledException(UnhandledException exception)
         {
+#if PROTO_PROMISE_DEVELOPER_MODE
+            exception = new UnhandledExceptionInternal(exception.Value, "Unhandled Exception added at (stacktrace in this exception)", new StackTrace(1, true).ToString(), exception);
+#endif
             _unhandledExceptionsLocker.Enter();
             _unhandledExceptions.Push(exception);
             _unhandledExceptionsLocker.Exit();

ProtoPromise_Unity/Assets/Plugins/ProtoPromise/Core/InternalShared/InterfacesInternal.cs

@@ -1,25 +1,10 @@
 using System;
-using System.Runtime.CompilerServices;
 
 namespace Proto.Promises
 {
     internal static partial class Internal
     {
-        // Abstract classes are used instead of interfaces, because virtual calls on interfaces are twice as slow as virtual calls on classes.
-        internal abstract partial class HandleablePromiseBase : ILinked<HandleablePromiseBase>
-        {
-            HandleablePromiseBase ILinked<HandleablePromiseBase>.Next
-            {
-                [MethodImpl(InlineOption)]
-                get { return _next; }
-                [MethodImpl(InlineOption)]
-                set { _next = value; }
-            }
-
-            internal abstract void Handle(ref ExecutionScheduler executionScheduler);
-            internal abstract void MakeReady(PromiseRef owner, ValueContainer valueContainer, ref ExecutionScheduler executionScheduler);
-        }
-
+        // Abstract class is used instead of interface, because virtual calls on interfaces are twice as slow as virtual calls on classes.
         internal abstract class ValueContainer
         {
             internal abstract void Retain();
@@ -29,6 +14,7 @@ internal abstract class ValueContainer
             internal abstract object Value { get; }
 
             internal abstract void ReleaseAndMaybeAddToUnhandledStack(bool shouldAdd);
+            internal abstract void AddToUnhandledStack();
         }
 
         internal partial interface ITraceable { }

ProtoPromise_Unity/Assets/Plugins/ProtoPromise/Core/InternalShared/PoolInternal.cs

@@ -10,6 +10,8 @@
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 
+[assembly: InternalsVisibleTo("ProtoPromiseTests")]
+
 namespace Proto.Promises
 {
     partial class Internal
@@ -74,52 +76,68 @@ internal static void Repool(TLinked obj)
             internal static T TryTake<T>() where T : class, TLinked
             {
                 TLinked obj = Type<T>.TryTake();
-                RemoveFromTrackedObjects(obj);
+#if PROMISE_DEBUG
+                if (_trackObjectsForRelease & obj == null)
+                {
+                    // Create here via reflection so that the object can be tracked.
+                    obj = (TLinked) Activator.CreateInstance(typeof(T), true);
+                }
+#endif
+                MarkNotInPool(obj);
                 return (T) obj;
             }
 
             [MethodImpl(InlineOption)]
             internal static void MaybeRepool<T>(T obj) where T : class, TLinked
             {
+                MarkInPool(obj);
                 if (Promise.Config.ObjectPoolingEnabled)
                 {
-                    AddToTrackedObjects(obj);
                     Type<T>.Repool(obj);
                 }
                 else
                 {
+                    // Finalizers are only used to validate that objects were used and released properly.
+                    // If the object is being repooled, it means it was released properly. If pooling is disabled, we don't need the finalizer anymore.
+                    // SuppressFinalize reduces pressure on the system when the GC runs.
                     GC.SuppressFinalize(obj);
                 }
             }
 
-            static partial void AddToTrackedObjects(object obj);
-            static partial void RemoveFromTrackedObjects(object obj);
+            static partial void MarkInPool(object obj);
+            static partial void MarkNotInPool(object obj);
 #if PROMISE_DEBUG
-
-            static partial void AddToTrackedObjects(object obj)
+            static partial void MarkInPool(object obj)
             {
                 lock (_pooledObjects)
                 {
-                    if (!_pooledObjects.Add(obj))
+                    if (Promise.Config.ObjectPoolingEnabled && !_pooledObjects.Add(obj))
                     {
                         throw new Exception("Same object was added to the pool twice: " + obj);
                     }
+                    _inUseObjects.Remove(obj);
                 }
             }
 
-            static partial void RemoveFromTrackedObjects(object obj)
+            static partial void MarkNotInPool(object obj)
             {
                 lock (_pooledObjects)
                 {
                     _pooledObjects.Remove(obj);
+                    if (_trackObjectsForRelease && !_inUseObjects.Add(obj))
+                    {
+                        throw new Exception("Same object was taken from the pool twice: " + obj);
+                    }
                 }
             }
 #endif
         }
 
         static partial void ThrowIfInPool(object obj);
 #if PROMISE_DEBUG
+        private static bool _trackObjectsForRelease = false;
         private static readonly HashSet<object> _pooledObjects = new HashSet<object>();
+        private static readonly HashSet<object> _inUseObjects = new HashSet<object>();
 
         static Internal()
         {
@@ -142,7 +160,34 @@ static Internal()
                 }
             }
         }
+
+        // This is used in unit testing, because finalizers are not guaranteed to run, even when calling `GC.WaitForPendingFinalizers()`.
+        internal static void TrackObjectsForRelease()
+        {
+            _trackObjectsForRelease = true;
+        }
+
+        internal static void AssertAllObjectsReleased()
+        {
+            lock (_pooledObjects)
+            {
+                if (_inUseObjects.Count > 0)
+                {
+                    System.Text.StringBuilder sb = new System.Text.StringBuilder();
+                    sb.AppendLine("Objects not released:");
+                    sb.AppendLine();
+                    ITraceable traceable = null;
+                    foreach (var obj in _inUseObjects)
+                    {
+                        traceable = traceable ?? obj as ITraceable;
+                        sb.AppendLine(obj.ToString());
+                        GC.SuppressFinalize(obj); // SuppressFinalize to not spoil the results of subsequent unit tests.
+                    }
+                    _inUseObjects.Clear();
+                    throw new UnreleasedObjectException(sb.ToString(), GetFormattedStacktrace(traceable));
+                }
+            }
+        }
 #endif
     }
-}
-
+}