Jest and why-is-node-running reports open handle on exit due to InstanceData.drop_queue's threadsafe function

[mjameswh]

Description

In Napi-6 mode, InstanceData registers a drop_queue threadsafe function, which ensures that leaked resources correctly get disposed. However, that TSFN itself is being reported by Jest, why-is-node-running and others (simply "Jest" thereafter) as leaked resources, since that function is never destroyed.

How to reproduce

InstanceData is lazily initialized. The issue can be observed by running any Jest test that forces initialization of InstanceData, notably by creating a Root object or calling cx.channel(). For example:

In rust:

pub fn test_inner_data_drop_queue_leak(mut cx: FunctionContext) -> JsResult<JsObject> {
    let some_object = cx.empty_object().root(&mut cx).into_inner(&mut cx);
    Ok(some_object)
}

In JavaScript:

test('httpWorkflow with mock activity', () => {
      (native as any).testInnerDataDropQueueLeak();
});

Then running using:

npx jest --detectOpenHandles

Results in this Jest warning message:

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  neon threadsafe function

Discussion and Proposed solution

This is technically a false-positive, since the TSFN is .unref() so that it will not keep the process alive. Unfortunately, Node's async hook callbacks API doesn't provide an official way to know that an async resource has been unref(). There is therefore no way for Jest to know that the TSFN can safely be ignored, by relying on Node's API alone.

Instead, Jest looks at the async resource for the presence of a hasRef() function. If that function exists on the async resource object, then Jest will call that function to determine if the resource is still active. why-is-node-running also support that function, and Node itself has added this function on some of its own resources specifically for the purpose of helping Jest and the like properly detect leaks.

It is therefore suggested that a hasRef() function should be added to the async resource associated TSFNs. That function would simply return either true or false, depending on either the TSFN is referenced() or unref().

[kjvalencik]

Thanks for sharing this detailed report @mjameswh!

Neon does not currently provide an async_resource when creating a Threadsafe function. That gives Neon an opportunity to add this functionality. We need to ensure it is fairly lightweight since there is not currently a use case beyond helping to remove this false positive. This shouldn't be too much of a concern since creating threadsafe function must be infrequent for a high performing add-on.

One option:

• Create a hasRef JsFunction and keep a copy of it in InstanceData
• Add a is_ref: Arc<Mutex<bool>> to ThreadsafeFunction
• Mutate the is_ref when ref and unref is called
• Create an object to use as async_resource in each ThreadsafeFunction
• Use napi_wrap to attach is_ref to the async resource
• Add a hasRef key to a function that will use napi_unwrap on this and check if it is referenced

This will allow tools that check for hasRef to be able to tell if Neon threadsafe functions are referenced.