Crash on Electron shutdown

[SuibianP]

During Electron shutdown, the finalizeCallback of pending ThreadSafeFunction is automatically triggered without explicit Release. It seems that NAPI calls at that time can fail with error 10 (napi_pending_exception), and the node-addon-wrapper C++ wrapper turns that into C++ exception. I also found this comment.

Interestingly, this issue only happens during Cmd-Q shutdown of Electron, but not if Ctrl-C'd.

#include "ThreadSafeDeferred.hpp"

Napi::Value myFunctionBoundToJS(const Napi::CallbackInfo & info) {
	auto myDeferred = new ThreadSafeDeferred(info.Env());
	return myDeferred->Promise();
}

Napi::Object Init(Napi::Env env, Napi::Object exports) {
	exports.Set(Napi::String::New(env, "hello"),
			Napi::Function::New(env, myFunctionBoundToJS));
	return exports;
}

NODE_API_MODULE(hello, Init)

Thread 0 Crashed:: CrBrowserMain Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	       0x193247720 __pthread_kill + 8
1   libsystem_pthread.dylib       	       0x19327ff70 pthread_kill + 288
2   libsystem_c.dylib             	       0x19318c908 abort + 128
3   libc++abi.dylib               	       0x19323644c abort_message + 132
4   libc++abi.dylib               	       0x193224a40 demangling_terminate_handler() + 348
5   libobjc.A.dylib               	       0x192ecd3f4 _objc_terminate() + 172
6   libc++abi.dylib               	       0x193235710 std::__terminate(void (*)()) + 16
7   libc++abi.dylib               	       0x193238cdc __cxxabiv1::failed_throw(__cxxabiv1::__cxa_exception*) + 88
8   libc++abi.dylib               	       0x193238c84 __cxa_throw + 92
9   test.node                     	       0x109bb50a4 Napi::Promise::Deferred::Reject(napi_value__*) const + 116 (napi-inl.h:2150)
10  test.node                     	       0x109bb4ec4 ThreadSafeDeferred::ThreadSafeDeferred(Napi::Env)::$_0::operator()(Napi::Env) const + 280 (ThreadSafeDeferred.cpp:21)
11  test.node                     	       0x109bb461c Napi::details::ThreadSafeFinalize<void, ThreadSafeDeferred::ThreadSafeDeferred(Napi::Env)::$_0, void>::Wrapper(napi_env__*, void*, void*) + 84 (napi-inl.h:219)
12  Electron Framework            	       0x10db4bfb0 node::ThreadPoolWork::ScheduleWork()::'lambda'(uv_work_s*, int)::__invoke(uv_work_s*, int) + 1312
13  Electron Framework            	       0x10db4be80 node::ThreadPoolWork::ScheduleWork()::'lambda'(uv_work_s*, int)::__invoke(uv_work_s*, int) + 1008
14  Electron Framework            	       0x10d87fde0 uv_run + 808
15  Electron Framework            	       0x10db15c9c node::Environment::CleanupHandles() + 184
16  Electron Framework            	       0x10db1628c node::Environment::RunCleanup() + 212
17  Electron Framework            	       0x10dab3ed0 node::FreeEnvironment(node::Environment*) + 156
18  Electron Framework            	       0x10da0e9e8 node::sqlite::UserDefinedFunction::xDestroy(void*) + 32268
19  Electron Framework            	       0x10d976308 v8::CpuProfileNode::GetNodeId() const + 154696
20  Electron Framework            	       0x10d35b04c v8::ObjectTemplate::SetAccessCheckCallbackAndHandler(bool (*)(v8::Local<v8::Context>, v8::Local<v8::Object>, v8::Local<v8::Value>), v8::NamedPropertyHandlerConfiguration const&, v8::IndexedPropertyHandlerConfiguration const&, v8::Local<v8::Value>) + 234028
21  Electron Framework            	       0x10d35aacc v8::ObjectTemplate::SetAccessCheckCallbackAndHandler(bool (*)(v8::Local<v8::Context>, v8::Local<v8::Object>, v8::Local<v8::Value>), v8::NamedPropertyHandlerConfiguration const&, v8::IndexedPropertyHandlerConfiguration const&, v8::Local<v8::Value>) + 232620
22  Electron Framework            	       0x10d0eba68 node::PrincipalRealm::domexception_function() const + 64584
23  Electron Framework            	       0x10d0eb920 node::PrincipalRealm::domexception_function() const + 64256
24  Electron Framework            	       0x10ccc78f0 v8::Isolate::SetAddCrashKeyCallback(void (*)(v8::CrashKeyId, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&)) + 80716
25  Electron Framework            	       0x10ccc4bcc v8::Isolate::SetAddCrashKeyCallback(void (*)(v8::CrashKeyId, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&)) + 69160
26  Electron Framework            	       0x10cfbf3fc v8::Object::SetIntegrityLevel(v8::Local<v8::Context>, v8::IntegrityLevel) + 120452
27  Electron Framework            	       0x10cfbed20 v8::Object::SetIntegrityLevel(v8::Local<v8::Context>, v8::IntegrityLevel) + 118696
28  Electron Framework            	       0x10d891b2c ElectronMain + 124
29  dyld                          	       0x192f00274 start + 2840

 # Call Site
00 ntdll!NtDelayExecution
01 ntdll!RtlDelayExecution
02 KERNELBASE!SleepEx
03 electron!crashpad::`anonymous namespace'::UnhandledExceptionHandler
04 KERNELBASE!UnhandledExceptionFilter
05 ntdll!RtlUserThreadStart$filt$0
06 ntdll!_C_specific_handler
07 ntdll!RtlpExecuteHandlerForException
08 ntdll!RtlDispatchException
09 ntdll!RtlRaiseException
0a KERNELBASE!RaiseException
0b ThreadSafeDeferredUser!_CxxThrowException
0c ThreadSafeDeferredUser!Napi::Promise::Deferred::Reject
0d ThreadSafeDeferredUser!<lambda_82c65d5f369bdde9fbf5d46df6704c3d>::operator()
0e ThreadSafeDeferredUser!Napi::details::ThreadSafeFinalize<void,<lambda_82c65d5f369bdde9fbf5d46df6704c3d>,Job>::FinalizeWrapperWithData
0f electron!node_napi_env__::CallFinalizer<0>::<lambda_1>::operator()
10 electron!napi_env__::CallIntoModule<`lambda at ..\..\third_party\electron_node\src\node_api.cc:48:7' &,`lambda at ..\..\third_party\electron_node\src\node_api.cc:89:24'>
11 electron!node_napi_env__::CallbackIntoModule
12 electron!node_napi_env__::CallFinalizer
13 electron!v8impl::`anonymous namespace'::ThreadSafeFunction::Finalize
14 electron!v8impl::`anonymous namespace'::ThreadSafeFunction::CloseHandlesAndMaybeDelete::<lambda_1>::operator()
15 electron!node::Environment::CloseHandle<uv_handle_s,`lambda at ..\..\third_party\electron_node\src\node_api.cc:450:9'>::<lambda_1>::operator()
16 electron!node::Environment::CloseHandle<uv_handle_s,`lambda at ..\..\third_party\electron_node\src\node_api.cc:450:9'>::<lambda_1>::__invoke
17 electron!uv__process_endgames
18 electron!uv_run
19 electron!node::Environment::CleanupHandles
1a electron!node::Environment::RunCleanup
1b electron!node::FreeEnvironment
1c electron!electron::NodeBindings::CreateEnvironment::<lambda_0>::operator()
1d electron!std::__Cr::__shared_ptr_pointer<node::Environment *,`lambda at ..\..\electron\shell\common\node_bindings.cc:765:22',std::__Cr::allocator<node::Environment> >::__on_zero_shared
1e electron!std::__Cr::__shared_count::__release_shared
1f electron!std::__Cr::__shared_weak_count::__release_shared
20 electron!std::__Cr::shared_ptr<node::Environment>::~shared_ptr
21 electron!std::__Cr::shared_ptr<node::Environment>::reset
22 electron!electron::ElectronBrowserMainParts::PostMainMessageLoopRun
23 electron!content::BrowserMainLoop::ShutdownThreadsAndCleanUp
24 electron!content::BrowserMainRunnerImpl::Shutdown
25 electron!content::BrowserMain
26 electron!content::RunBrowserProcessMain
27 electron!content::ContentMainRunnerImpl::RunBrowser
28 electron!content::ContentMainRunnerImpl::Run
29 electron!content::RunContentProcess
2a electron!content::ContentMain
2b electron!wWinMain
2c electron!invoke_main
2d electron!__scrt_common_main_seh
2e KERNEL32!BaseThreadInitThunk
2f ntdll!RtlUserThreadStart

[SuibianP]

diff --git a/src/ThreadSafeDeferred.cpp b/src/ThreadSafeDeferred.cpp
index 01b808f..4a7577a 100644
--- a/src/ThreadSafeDeferred.cpp
+++ b/src/ThreadSafeDeferred.cpp
@@ -17,9 +17,10 @@ ThreadSafeDeferred::ThreadSafeDeferred(const Napi::Env env)
 			} else {
 				Napi::Promise::Deferred::Resolve(this->createValueCb(env));
 			}
-		} else {
+		} else if (this->fate == EFate::REJECTED) {
 			Napi::Promise::Deferred::Reject(Napi::Error::New(env, this->errorMsg).Value());
 		}
+		// If fate is UNRESOLVED, tsf is being called due to environment teardown
 		try {
 			delete this;
 		} catch (const std::runtime_error&) {

should fix that for most cases. The crash might still happen if environment teardown happens after fate is set but before the finalizer is run. An atomic indicator set in napi_add_env_cleanup_hook would help but might be overkill.

[0815fox]

I have to admit, that I actually didn't even remember I made this. Would you want to take this thing over, I'd happily hand it over to you?