Skip to content

Commit b8667af

Browse files
committed
Add first version of node / python deadlock fix.
1 parent 0764310 commit b8667af

File tree

3 files changed

+67
-15
lines changed

3 files changed

+67
-15
lines changed

source/loaders/node_loader/include/node_loader/node_loader_impl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ NODE_LOADER_NO_EXPORT void node_loader_impl_exception(napi_env env, napi_status
5555

5656
NODE_LOADER_NO_EXPORT void node_loader_impl_finalizer(napi_env env, napi_value v, void *data);
5757

58+
NODE_LOADER_NO_EXPORT void node_loader_impl_handle_promise(loader_impl_node node_impl, napi_env env, napi_deferred deferred, void *result, napi_status (*deferred_fn)(napi_env, napi_deferred, napi_value), const char error_str[]);
59+
5860
NODE_LOADER_NO_EXPORT value node_loader_impl_napi_to_value(loader_impl_node node_impl, napi_env env, napi_value recv, napi_value v);
5961

6062
NODE_LOADER_NO_EXPORT napi_value node_loader_impl_value_to_napi(loader_impl_node node_impl, napi_env env, value arg);

source/loaders/node_loader/source/node_loader_impl.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,18 @@ struct loader_impl_async_future_delete_safe_type
513513
node_impl(node_impl), f(f), node_future(node_future) {}
514514
};
515515

516+
struct loader_impl_async_handle_promise_safe_type
517+
{
518+
loader_impl_node node_impl;
519+
napi_deferred deferred;
520+
void *result;
521+
napi_status (*deferred_fn)(napi_env, napi_deferred, napi_value);
522+
const char *error_str;
523+
524+
loader_impl_async_handle_promise_safe_type(loader_impl_node node_impl, napi_deferred deferred, void *result, napi_status (*deferred_fn)(napi_env, napi_deferred, napi_value), const char error_str[]) :
525+
node_impl(node_impl), deferred(deferred), result(result), deferred_fn(deferred_fn), error_str(error_str) {}
526+
};
527+
516528
struct loader_impl_async_destroy_safe_type
517529
{
518530
loader_impl_node node_impl;
@@ -540,6 +552,7 @@ struct loader_impl_node_type
540552
loader_impl_threadsafe_type<loader_impl_async_func_destroy_safe_type> threadsafe_func_destroy;
541553
loader_impl_threadsafe_type<loader_impl_async_future_await_safe_type> threadsafe_future_await;
542554
loader_impl_threadsafe_type<loader_impl_async_future_delete_safe_type> threadsafe_future_delete;
555+
loader_impl_threadsafe_type<loader_impl_async_handle_promise_safe_type> threadsafe_handle_promise;
543556
loader_impl_threadsafe_type<loader_impl_async_destroy_safe_type> threadsafe_destroy;
544557

545558
uv_thread_t thread;
@@ -647,6 +660,8 @@ static value node_loader_impl_discover_function_safe(napi_env env, loader_impl_a
647660

648661
static void node_loader_impl_discover_safe(napi_env env, loader_impl_async_discover_safe_type *discover_safe);
649662

663+
static void node_loader_impl_handle_promise_safe(napi_env env, loader_impl_async_handle_promise_safe_type *handle_promise_safe);
664+
650665
static void node_loader_impl_destroy_safe(napi_env env, loader_impl_async_destroy_safe_type *destroy_safe);
651666

652667
static char *node_loader_impl_get_property_as_char(napi_env env, napi_value obj, const char *prop);
@@ -2493,6 +2508,7 @@ void node_loader_impl_clear_safe(napi_env env, loader_impl_async_clear_safe_type
24932508
napi_status status = napi_open_handle_scope(env, &handle_scope);
24942509

24952510
node_loader_impl_exception(env, status);
2511+
24962512
/* Get function table object from reference */
24972513
status = napi_get_reference_value(env, clear_safe->node_impl->function_table_object_ref, &function_table_object);
24982514

@@ -3271,6 +3287,30 @@ void node_loader_impl_discover_safe(napi_env env, loader_impl_async_discover_saf
32713287
node_loader_impl_exception(env, status);
32723288
}
32733289

3290+
void node_loader_impl_handle_promise_safe(napi_env env, loader_impl_async_handle_promise_safe_type *handle_promise_safe)
3291+
{
3292+
napi_handle_scope handle_scope;
3293+
3294+
/* Create scope */
3295+
napi_status status = napi_open_handle_scope(env, &handle_scope);
3296+
3297+
node_loader_impl_exception(env, status);
3298+
3299+
/* Convert MetaCall value to napi value and call resolve or reject of NodeJS */
3300+
napi_value js_result = node_loader_impl_value_to_napi(handle_promise_safe->node_impl, env, handle_promise_safe->result);
3301+
status = handle_promise_safe->deferred_fn(env, handle_promise_safe->deferred, js_result);
3302+
3303+
if (status != napi_ok)
3304+
{
3305+
napi_throw_error(env, nullptr, handle_promise_safe->error_str);
3306+
}
3307+
3308+
/* Close scope */
3309+
status = napi_close_handle_scope(env, handle_scope);
3310+
3311+
node_loader_impl_exception(env, status);
3312+
}
3313+
32743314
#if defined(_WIN32) && defined(_MSC_VER) && (_MSC_VER >= 1200)
32753315
/* TODO: _Ret_maybenull_ HMODULE WINAPI GetModuleHandleW(_In_opt_ LPCWSTR lpModuleName); */
32763316
_Ret_maybenull_ HMODULE WINAPI get_module_handle_a_hook(_In_opt_ LPCSTR lpModuleName)
@@ -3361,6 +3401,7 @@ void *node_loader_impl_register(void *node_impl_ptr, void *env_ptr, void *functi
33613401
node_impl->threadsafe_func_destroy.initialize(env, "node_loader_impl_async_func_destroy_safe", &node_loader_impl_func_destroy_safe);
33623402
node_impl->threadsafe_future_await.initialize(env, "node_loader_impl_async_future_await_safe", &node_loader_impl_future_await_safe);
33633403
node_impl->threadsafe_future_delete.initialize(env, "node_loader_impl_async_future_delete_safe", &node_loader_impl_future_delete_safe);
3404+
node_impl->threadsafe_handle_promise.initialize(env, "node_loader_impl_async_handle_promise_safe", &node_loader_impl_handle_promise_safe);
33643405
node_impl->threadsafe_destroy.initialize(env, "node_loader_impl_async_destroy_safe", &node_loader_impl_destroy_safe);
33653406
}
33663407

@@ -4030,6 +4071,23 @@ int node_loader_impl_discover(loader_impl impl, loader_handle handle, context ct
40304071
return discover_safe.result;
40314072
}
40324073

4074+
void node_loader_impl_handle_promise(loader_impl_node node_impl, napi_env env, napi_deferred deferred, void *result, napi_status (*deferred_fn)(napi_env, napi_deferred, napi_value), const char error_str[])
4075+
{
4076+
loader_impl_async_handle_promise_safe_type handle_promise_safe(node_impl, deferred, result, deferred_fn, error_str);
4077+
4078+
/* Check if we are in the JavaScript thread */
4079+
if (node_impl->js_thread_id == std::this_thread::get_id())
4080+
{
4081+
/* We are already in the V8 thread, we can call safely */
4082+
node_loader_impl_handle_promise_safe(env, &handle_promise_safe);
4083+
}
4084+
else
4085+
{
4086+
/* Submit the task to the async queue */
4087+
loader_impl_threadsafe_invoke_type<loader_impl_async_handle_promise_safe_type> invoke(node_impl->threadsafe_handle_promise, handle_promise_safe);
4088+
}
4089+
}
4090+
40334091
#define container_of(ptr, type, member) \
40344092
(type *)((char *)(ptr) - (char *)&((type *)0)->member)
40354093

@@ -4270,6 +4328,7 @@ void node_loader_impl_destroy_safe_impl(loader_impl_node node_impl, napi_env env
42704328
node_impl->threadsafe_func_destroy.abort(env);
42714329
node_impl->threadsafe_future_await.abort(env);
42724330
node_impl->threadsafe_future_delete.abort(env);
4331+
node_impl->threadsafe_handle_promise.abort(env);
42734332
}
42744333

42754334
/* Clear persistent references */

source/loaders/node_loader/source/node_loader_port.cpp

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -198,29 +198,23 @@ napi_value node_loader_port_metacall_await(napi_env env, napi_callback_info info
198198
ctx->env = env;
199199

200200
auto resolve = [](void *result, void *data) -> void * {
201+
static const char promise_error_str[] = "Failed to resolve the promise";
202+
201203
promise_context_type *ctx = static_cast<promise_context_type *>(data);
202-
napi_value js_result = node_loader_impl_value_to_napi(ctx->node_impl, ctx->env, result);
203-
napi_status status = napi_resolve_deferred(ctx->env, ctx->deferred, js_result);
204204

205-
if (status != napi_ok)
206-
{
207-
napi_throw_error(ctx->env, nullptr, "Failed to resolve the promise");
208-
}
205+
node_loader_impl_handle_promise(ctx->node_impl, ctx->env, ctx->deferred, result, &napi_resolve_deferred, promise_error_str);
209206

210207
delete ctx;
211208

212209
return NULL;
213210
};
214211

215212
auto reject = [](void *result, void *data) -> void * {
213+
static const char promise_error_str[] = "Failed to reject the promise";
214+
216215
promise_context_type *ctx = static_cast<promise_context_type *>(data);
217-
napi_value js_result = node_loader_impl_value_to_napi(ctx->node_impl, ctx->env, result);
218-
napi_status status = napi_reject_deferred(ctx->env, ctx->deferred, js_result);
219216

220-
if (status != napi_ok)
221-
{
222-
napi_throw_error(ctx->env, nullptr, "Failed to reject the promise");
223-
}
217+
node_loader_impl_handle_promise(ctx->node_impl, ctx->env, ctx->deferred, result, &napi_reject_deferred, promise_error_str);
224218

225219
delete ctx;
226220

@@ -230,15 +224,12 @@ napi_value node_loader_port_metacall_await(napi_env env, napi_callback_info info
230224
/* Await to the function */
231225
void *ret = metacall_await_s(name, args, argc - 1, resolve, reject, ctx);
232226

233-
/* TODO: Is this needed? */
234-
/*
235227
if (metacall_value_id(ret) == METACALL_THROWABLE)
236228
{
237229
napi_value result = node_loader_impl_value_to_napi(node_impl, env, ret);
238230

239231
napi_throw(env, result);
240232
}
241-
*/
242233

243234
/* Release current reference of the environment */
244235
// node_loader_impl_env(node_impl, nullptr);

0 commit comments

Comments
 (0)