Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/node_misc.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ and `node::MakeCallback`. Using this mechanism to call back into JavaScript, as
`v8::Function::Call` ensures that the callback is executed in the correct async context. This ensures that async mechanisms
such as domains and [async_hooks][] function correctly.

`makeCallBackScope` is a factory for `node::callbackScope`. A `callbackScope` ensures that micro-tasks such as `Pormise` handlers and `nextTick` callbacks will be executed after the function returns. It is a RAII guard and must always be allocated on the stack. This must be used when an async worker does an action with an effect on the event loop such as resolving a `Promise` without calling JavaScript - otherwise `makeCallback` already takes care of the micro-tasks queue.

Definition:

```c++
Expand All @@ -39,6 +41,9 @@ class AsyncResource {
const char* method,
int argc,
v8::Local<v8::Value>* argv);

node::CallbackScope node::CallbackScope makeCallbackScope(
v8::Local<v8::Object> target);
};
```

Expand Down
6 changes: 6 additions & 0 deletions nan.h
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,12 @@ class AsyncResource {
#endif
}

#if NODE_MODULE_VERSION >= NODE_9_0_MODULE_VERSION
inline node::CallbackScope makeCallbackScope(v8::Local<v8::Object> target) {
return {v8::Isolate::GetCurrent(), target, context};
}
#endif

@Flarna Flarna Sep 25, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this require an ifdef for all users supporting ancient node.js versions if there is no dummy implementation for these older node.js versions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an #ifdef.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When it comes to the users, the users do not have any choice as this does not work the same way before Node.js 9.0.0. There are no AsyncResources at all.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But here AsyncResource does exist for all versions of Node, it just does not perform the non-applicable operations on old versions, leading to the same observable end result.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why is the typecast needed now? ::context is a node::async_context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that node::CallbackScope does not exist, so the user will have to use an ifdef.

The only way that I see around this is to wrap the whole construction and add a Nan::CallbackScope that is an empty object on Node <9.0.0.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think I understand. Introducing a dummy Nan::CallbackScope is fine, but would it not make sense to have it just do a Nan::MakeCallback for old versions, so the function at least gets called. Am I right in thinking that on versions of Node before 9.0, the closest match would be to just run the callback and call it a day?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nan::CallbackScope is just a RIAA guard that means to run the micro tasks when the scope is exited. I wonder how did this work before Node 9.0.0 which introduced the async contexts, 'CallbackScopeandMakeCallback`.

I am not very sure if before Node 9, binary modules had to run themselves the micro tasks or not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the proposal is to add an #error in this case.

The rationale is that this is something modules will have to add now in order to work correctly on Node 26+, while dropping support for Node 8 and earlier.

Anyway, I am afraid that there is no way to safely implement this on Node 8 - this was before Promise was common and all C++ modules used callbacks which do not have this problem.


private:
NAN_DISALLOW_ASSIGN_COPY_MOVE(AsyncResource)
#if NODE_MODULE_VERSION >= NODE_9_0_MODULE_VERSION
Expand Down
4 changes: 4 additions & 0 deletions test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@
"target_name" : "asyncresource"
, "sources" : [ "cpp/asyncresource.cpp" ]
}
, {
"target_name" : "asyncresource-promise"
, "sources" : [ "cpp/asyncresource-promise.cpp" ]
}
, {
"target_name" : "callbackcontext"
, "sources" : [ "cpp/callbackcontext.cpp" ]
Expand Down
73 changes: 73 additions & 0 deletions test/cpp/asyncresource-promise.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*********************************************************************
* NAN - Native Abstractions for Node.js
*
* Copyright (c) 2018 NAN contributors
*
* MIT License <https://github.com/nodejs/nan/blob/master/LICENSE.md>
********************************************************************/

#include <nan.h>

#include "sleep.h" // NOLINT(build/include_subdir)

using namespace Nan; // NOLINT(build/namespaces)

class DelayRequest : public AsyncResource {
public:
DelayRequest(int milliseconds_, v8::Local<v8::Promise::Resolver> resolver_)
: AsyncResource("nan:test.DelayPromise"),
milliseconds(milliseconds_) {
resolver.Reset(resolver_);
request.data = this;
}
~DelayRequest() {
resolver.Reset();
}

Persistent<v8::Promise::Resolver> resolver;
uv_work_t request;
int milliseconds;
};

void Delay(uv_work_t* req) {
DelayRequest *delay_request = static_cast<DelayRequest*>(req->data);
Sleep(delay_request->milliseconds);
}

void AfterDelay(uv_work_t* req, int status) {
HandleScope scope;

DelayRequest *delay_request = static_cast<DelayRequest*>(req->data);
v8::Local<v8::Promise::Resolver> resolver = New(delay_request->resolver);
resolver->Resolve(Nan::GetCurrentContext(), New<v8::Boolean>(true));

v8::Local<v8::Object> target = New<v8::Object>();

// Run the callback in the async context.
node::CallbackScope callback_scope = delay_request->makeCallbackScope(target);

delete delay_request;
}

NAN_METHOD(Delay) {
int delay = To<int>(info[0]).FromJust();
v8::Local<v8::Promise::Resolver> resolver =
v8::Promise::Resolver::New(Nan::GetCurrentContext()).ToLocalChecked();

DelayRequest* delay_request = new DelayRequest(delay, resolver);

info.GetReturnValue().Set(resolver->GetPromise());

uv_queue_work(
GetCurrentEventLoop()
, &delay_request->request
, Delay
, reinterpret_cast<uv_after_work_cb>(AfterDelay));
}

NAN_MODULE_INIT(Init) {
Set(target, New<v8::String>("delay").ToLocalChecked(),
GetFunction(New<v8::FunctionTemplate>(Delay)).ToLocalChecked());
}

NODE_MODULE(asyncresource_promise, Init)
54 changes: 54 additions & 0 deletions test/js/asyncresource-promise-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*********************************************************************
* NAN - Native Abstractions for Node.js
*
* Copyright (c) 2026 NAN contributors
*
* MIT License <https://github.com/nodejs/nan/blob/master/LICENSE.md>
********************************************************************/

const version = process.versions.node.split('.');
if (version[0] < 9) {
process.exit(0);
}

try {
require('async_hooks');
} catch (e) {
process.exit(0);
}

const test = require('tap').test
, testRoot = require('path').resolve(__dirname, '..')
, delay = require('bindings')({ module_root: testRoot, bindings: 'asyncresource-promise' }).delay
, asyncHooks = require('async_hooks');

test('asyncresource-promise', function (t) {
t.plan(2);

var beforeCalled = false;
var promiseResolveCalled = false;

var hooks = asyncHooks.createHook({
init: function (asyncId, type, triggerAsyncId, resource) {
if (type === 'nan:test.DelayPromise') {
resourceAsyncId = asyncId;
}
},
before: function (asyncId) {
if (asyncId === resourceAsyncId) {
beforeCalled = true;
}
},
promiseResolve: function (asyncId) {
promiseResolveCalled = true;
}

});
hooks.enable();

originalExecutionAsyncId = asyncHooks.executionAsyncId();
delay(1000).then(function() {
t.ok(beforeCalled, 'before should have been called');
t.ok(promiseResolveCalled, 'promiseResolve should have been called');
})
});
Loading