timcassell / ProtoPromise

Robust and efficient library for management of asynchronous operations in C#/.Net.
MIT License
147 stars 13 forks source link

Add `Promise.Manager.ResetRuntimeContext()` API #303

Closed timcassell closed 9 months ago

timcassell commented 10 months ago

Fixes #283

timcassell commented 10 months ago

@drew-512 Want to give this a try?

drew-512 commented 10 months ago

Hi Tim!

Been going about 6-7 hours of dev time with this branch (about 2-4 runs per hour, arena reload features all disabled). The good news is I don't get the Promise not handled exception as raised previously.

However, 1 in 5 runs I get a Unity crash where the Unity crash reporter comes up etc. I enabled PROTO_PROMISE_DEVELOPER_MODE and PROTO_PROMISE_DEBUG_ENABLE and no messages or change.

Happy to help support further, just lmk.

timcassell commented 10 months ago

Are you able to share the crash log?

drew-512 commented 10 months ago

14-Nov.zip

Sure but not seeing anything in it. Best guess is the crash is happening before anything is written.

Not sure about anything. This project is a AV player so there's a lot of stuff happening and I don't want to implicate PP just yet. These crashes happen as the Unity debugger does suck.

timcassell commented 10 months ago

It looks like the crash is coming from something called Nova.

Log

``` cmd Application is shutting down... Cleanup mono info: Unity.ILPP.Runner.PostProcessingAssemblyLoadContext[0] ALC ILPP context 1 is unloading System Nova.Internal.DataStoreSystem Dispose failed with UnityEngine.UnityException: DestroyTransformAccessArray is not allowed to be called while application is terminating. at (wrapper managed-to-native) UnityEngine.Jobs.TransformAccessArray.DestroyTransformAccessArray(intptr) at UnityEngine.Jobs.TransformAccessArray.Dispose () [0x0001a] in /Users/bokken/build/output/unity/unity/Runtime/Transform/ScriptBindings/TransformAccessArray.bindings.cs:289 at Nova.Internal.Core.TransformDataStore`2[TDataStore,T].Dispose () [0x0002c] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Core/TransformDataStore.cs:221 at Nova.Internal.Layouts.LayoutDataStore.Dispose () [0x00001] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Layouts/LayoutDataStore.cs:758 at Nova.Internal.DataStoreSystem.Dispose () [0x00006] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/DataStoreSystem.cs:20 at Nova.Internal.Core.System`1[T].DomainReloadStarted () [0x00002] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Core/System.cs:50 #0 GetStacktrace(int) #1 DebugStringToFile(DebugStringToFileData const&) #2 DebugLogHandler_CUSTOM_Internal_Log(LogType, LogOption, ScriptingBackendNativeStringPtrOpaque*, ScriptingBackendNativeObjectPtrOpaque*) #3 (Mono JIT Code) (wrapper managed-to-native) UnityEngine.DebugLogHandler:Internal_Log (UnityEngine.LogType,UnityEngine.LogOption,string,UnityEngine.Object) System Nova.Internal.EngineManager Dispose failed with UnityEngine.UnityException: DestroyBuffer is not allowed to be called while application is terminating. at (wrapper managed-to-native) UnityEngine.ComputeBuffer.DestroyBuffer(UnityEngine.ComputeBuffer) at UnityEngine.ComputeBuffer.Dispose (System.Boolean disposing) [0x00007] in /Users/bokken/build/output/unity/unity/Runtime/Export/Shaders/ComputeBuffer.bindings.cs:44 at UnityEngine.ComputeBuffer.Dispose () [0x00001] in /Users/bokken/build/output/unity/unity/Runtime/Export/Shaders/ComputeBuffer.bindings.cs:35 at Nova.Internal.Rendering.ShaderBuffer.Dispose () [0x0000f] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Rendering/ShaderBuffer.cs:205 at Nova.Internal.Rendering.ShaderBufferUtils.SafeDispose (Nova.Internal.Rendering.ShaderBuffer shaderBuffer) [0x0000a] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Rendering/ShaderBuffer.cs:231 at Nova.Internal.Rendering.BatchGroupRenderer.Dispose () [0x0000d] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Rendering/BatchGroupRenderer.cs:404 at Nova.Internal.Utilities.Extensions.DictionaryExtensions.DisposeValues[K,V] (System.Collections.Generic.Dictionary`2[TKey,TValue] dict) [0x00019] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Utilities/Extensions/DictionaryExtensions.cs:43 at Nova.Internal.Rendering.RenderEngine.Dispose () [0x00097] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Rendering/RenderEngine.cs:619 at Nova.Internal.EngineManager.Dispose () [0x00031] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/EngineManager.cs:438 at Nova.Internal.Core.System`1[T].DomainReloadStarted () [0x00002] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Core/System.cs:50 #0 GetStacktrace(int) #1 DebugStringToFile(DebugStringToFileData const&) #2 DebugLogHandler_CUSTOM_Internal_Log(LogType, LogOption, ScriptingBackendNativeStringPtrOpaque*, ScriptingBackendNativeObjectPtrOpaque*) #3 (Mono JIT Code) (wrapper managed-to-native) UnityEngine.DebugLogHandler:Internal_Log (UnityEngine.LogType,UnityEngine.LogOption,string,UnityEngine.Object) System Nova.Editor.SceneViewInput Dispose failed with UnityEngine.UnityException: Internal_SetHotControl is not allowed to be called while application is terminating. at (wrapper managed-to-native) UnityEngine.GUIUtility.Internal_SetHotControl(int) at UnityEngine.GUIUtility.set_hotControl (System.Int32 value) [0x00001] in /Users/bokken/build/output/unity/unity/Modules/IMGUI/GUIUtility.cs:116 at Nova.Editor.SceneViewInput.Dispose () [0x00030] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Editor/SceneViewInput.cs:432 at Nova.Internal.Core.System`1[T].DomainReloadStarted () [0x00002] in /Users/aomeara/git.arcspace/arcspace.unity-app/Assets/Plugins/Nova/Scripts/Internal/Core/System.cs:50 #0 GetStacktrace(int) #1 DebugStringToFile(DebugStringToFileData const&) #2 DebugLogHandler_CUSTOM_Internal_Log(LogType, LogOption, ScriptingBackendNativeStringPtrOpaque*, ScriptingBackendNativeObjectPtrOpaque*) #3 (Mono JIT Code) (wrapper managed-to-native) UnityEngine.DebugLogHandler:Internal_Log (UnityEngine.LogType,UnityEngine.LogOption,string,UnityEngine.Object) * Assertion at boehm-gc.c:1830, condition `type < HANDLE_TYPE_MAX' not met ```

[Edit] Actually, those just looks like regular exceptions. The crash part is much harder to read.

Details

```cmd Obtained 21 stack frames. #0 0x007fff7d4b22c2 in __pthread_kill #1 0x007fff7d41c6a6 in abort #2 0x00000189047c15 in monoeg_g_printv #3 0x0000018902e0f1 in mono_log_write_logfile #4 0x00000189047f77 in monoeg_g_logv_nofree #5 0x000001890480df in monoeg_assertion_message #6 0x00000189048117 in mono_assertion_message_unreachable #7 0x00000189024bdb in mono_gc_weak_link_add #8 0x0000018901ebf8 in ves_icall_System_GCHandle_GetTargetHandle #9 0x00000188f8611b in ves_icall_System_GCHandle_GetTargetHandle_raw #10 0x000002250238c9 in (wrapper managed-to-native) System.Runtime.InteropServices.GCHandle:GetTargetHandle (object,intptr,System.Runtime.InteropServices.GCHandleType) [{0x7ff1ab91fc60} + 0xc9] (0x225023800 0x2250239bc) [0x15bae5a80 - Unity Child Domain] #11 0x000001ec5f492c in (wrapper runtime-invoke) object:runtime_invoke_virtual_void__this__ (object,intptr,intptr,intptr) [{0x7ff18b1757f0} + 0x18c] (0x1ec5f47a0 0x1ec5f4b77) [0x15bae5a80 - Unity Child Domain] #12 0x0000018901df6b in mono_gc_run_finalize #13 0x0000018901fa97 in finalizer_thread #14 0x00000188fe6056 in start_wrapper_internal #15 0x00000188fe5efb in start_wrapper #16 0x000001890633ff in GC_inner_start_routine #17 0x00000189063397 in GC_start_routine #18 0x007fff7d56b2eb in _pthread_body #19 0x007fff7d56e249 in _pthread_start #20 0x007fff7d56a40d in thread_start ```

The finalizer_thread and GCHandle:GetTargetHandle does look like it's related to my changes here.

timcassell commented 10 months ago

@drew-512 I think I fixed the crash.

drew-512 commented 10 months ago

Been going ~6 hours now and zero crashes (has 3 or 4 by this time yesterday). Will report end of tomorrow or so!

timcassell commented 10 months ago

So, this works by storing WeakReferences of each object so that their finalizers can be suppressed by the call to ResetRuntimeContext. WeakReferences add overhead, so I disabled the finalizers entirely in RELEASE mode. This means if a promise is not awaited properly, and it's rejected, that information will be lost. That's unfortunate, but I can't think of another way to support disabled domain reloads without adding overhead.

[Edit] I have an idea... maybe I can add finalizers and WeakReferences in the RejectContainers where the performance is not critical. Unobserved promises (and unreleased other types of objects) will still be undetectable in RELEASE mode, but it should at least expose uncaught rejections.

drew-512 commented 10 months ago

Hi Tim,

Still zero crashes since that patch from yesterday.

Yeah, concur that any additional overhead in release mode is the least preferred path.

Maybe one idea is explicit promises could be flagged as "unobservable", which would basically mean that an exception isn't raised if they're not observed. I may be mistaken, but something like await Promise.SwitchToForeground(); would only ever be unobserved in an Editor shutdown scenario.

timcassell commented 9 months ago

Still zero crashes since that patch from yesterday.

Yay!

Yeah, concur that any additional overhead in release mode is the least preferred path.

Indeed. I started to implement adding detection to RejectContainers, but I realized I would have to mark them on every await, which adds overhead anyway. So I guess it'll just have to be that way in RELEASE mode. If a promise is not awaited or forgotten, it will only be detectable in DEBUG mode (including uncaught rejections).

Maybe one idea is explicit promises could be flagged as "unobservable", which would basically mean that an exception isn't raised if they're not observed. I may be mistaken, but something like await Promise.SwitchToForeground(); would only ever be unobserved in an Editor shutdown scenario.

The problem is any promise could be unobserved in that scenario. Promise.SwitchToForeground() just happened to be what you got.

timcassell commented 9 months ago

Closing this as it appears the issue was actually fixed in #306.