Initializing uv_async_t handle in wrapped c++ classes hangs async/await operations

[BSoD-Ultimate]

• Version: Node.js 10.16.0
• Platform: Windows 8.1 x64

I'm writing a C++ module for JavaScript's use. The library exports some C++ classes to the JavaScript code. There is a C++ class have to start a continuously-running thread, and the data has to be notified to the Javascript code. My thought is initializing a uv_async_t handle as one of the class members for waking up Node.js's main thread. The handle is initialized in the class' constructor function. When running the Javascript code for testing, the asynchronous notification has no problem. Then I rewrite my Javascript code to the async/await form; The program hangs after the promise.resolve function executes. The problem even occurs when I'm not running asynchronous functions provided by the c++ class as an instance of the class exists. Dramatically, When I pause the program, then immediately continue the execution, the next line of async code gets executed.

Here is my c++ code that could reproduce the problem:

#include <nan.h>
#include <memory>
#include <thread>

// wrapper for uv_async_t (maybe wrong?)
class uvAsyncEvent
{
public:
    uvAsyncEvent(void* context, uv_async_cb callback)
    {
        m_pAsyncHandle.reset(new uv_async_t());
        m_pAsyncHandle->data = context;
        uv_async_init(uv_default_loop(), m_pAsyncHandle.get(), callback);
    }
    ~uvAsyncEvent()
    {

    }
    void* GetContext() const
    {
        return m_pAsyncHandle->data;
    }
    void Notify()
    {
        uv_async_send(m_pAsyncHandle.get());
    }

private:
    struct Deleter
    {
        void operator()(uv_async_t* asyncEvent) const
        {
            if (asyncEvent)
            {
                uv_close((uv_handle_t*)asyncEvent, Deleter::uvCloseCallback);
            }
        }

        static void uvCloseCallback(uv_handle_t* handle)
        {
            delete (uv_async_t*)handle;
        }
    };
    std::unique_ptr<uv_async_t, Deleter> m_pAsyncHandle;
};

// a c++ class exported to Node.js which holds uv_async_t handles
class Hang
    : public Nan::ObjectWrap
{
public:
    static NAN_MODULE_INIT(Init)
    {
        v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
        tpl->SetClassName(Nan::New("Hang").ToLocalChecked());
        tpl->InstanceTemplate()->SetInternalFieldCount(1);

        constructor.Reset(Nan::GetFunction(tpl).ToLocalChecked());

        Nan::Set(target, Nan::New("Hang").ToLocalChecked(), Nan::GetFunction(tpl).ToLocalChecked());
    }

private:
    Hang()
        : m_pEvent(std::make_unique<uvAsyncEvent>(this, asyncCallback)) // async/await hangs
    {
    }
public:
    ~Hang()
    {
    }

    static NAN_METHOD(New)
    {
        if (info.IsConstructCall()) {
            // Invoked as constructor: `new Hang(...)`
            Hang* obj = new Hang();
            obj->Wrap(info.This());

            info.GetReturnValue().Set(info.This());
        }
        else {
            // Invoked as plain function `Hang(...)`, turn into construct call.
            const int argc = 1;
            v8::Local<v8::Value> argv[argc] = { info[0] };
            v8::Local<v8::Function> cons = Nan::New(constructor);
            v8::Local<v8::Object> result =
                Nan::NewInstance(cons, argc, argv).ToLocalChecked();
            info.GetReturnValue().Set(result);
        }
    }

private:
    static void asyncCallback(uv_async_t* handle)
    {

    }

private:
    std::unique_ptr<uvAsyncEvent> m_pEvent;


private:
    static Nan::Persistent<v8::Function> constructor;
};
Nan::Persistent<v8::Function> Hang::constructor;

// an asynchronous function for test
NAN_METHOD(testAsync)
{
    class Worker : public Nan::AsyncWorker
    {
    public:
        Worker(Nan::Callback* callbk)
            : Nan::AsyncWorker(callbk)
        {
        }

        void Execute() override
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(200));
        }

        void HandleOKCallback() override
        {
            Nan::HandleScope scope;

            int argc = 1;
            std::unique_ptr<v8::Local<v8::Value>[]> argv(new v8::Local<v8::Value>[argc]());

            argv[0] = Nan::New("async task result").ToLocalChecked();

            Nan::Call(*callback, argc, argv.get());
        }
    };

    Nan::Callback* callbk = new Nan::Callback(Nan::To<v8::Function>(info[0]).ToLocalChecked());

    Worker* pWorker = new Worker(callbk);
    Nan::AsyncQueueWorker(pWorker);
}

NAN_MODULE_INIT(InitAll) 
{
    Hang::Init(target);
    Nan::SetMethod(target, "testAsync", testAsync);
}

NODE_MODULE(hang, InitAll)

And the JavaScript code for test:

const {Hang, testAsync} = require('./build/Release/uv_async_hang');

let hang = new Hang();

function doTestAsync()
{
  return new Promise((resolve, reject) => {
    testAsync((data) => {
      // This line of code got executed without any problems
      // after 200ms of the function call(according to the C++ code)
      console.log(data);
      resolve(data);
    })
  });
}

// this is ok
//testAsync((data) => {
//    console.log(data);
//});

async function testHang()
{
  let asyncRet = await doTestAsync();

  // async/await operation blocks due to the creation of a "Hang" object
  // I paused the execution via the debugger, then continue, 
  // Dramatically this line gets executed
  console.log("after async call");
}


testHang();

Is this a bug or a known issue has to be avoided? If the latter one, any way to circumvent this?

I also reproduced this problem in Node.js 8.16.0 and 12.6.0 on my computer.

[bnoordhuis]

Can an admin move this to nodejs/help?

[sam-github]

I transfered. @bnoordhuis I think recent changes by github might have made it possible for anyone with write access to repos A and B to transfer issues between them. EDIT: its hard to tell for me because I have admin access to both repos.

[BSoD-Ultimate]

Update: On the computer in my office, the script blocks after the program output async task result, and will continue execution after I suspend then resume the script using the debugger(Visual Studio Code). Then the script enters into the state of hang as excepted since there is a uv_async_t handle held by a native C++ object. On the computer in my home, the script could output both async task result and after async call when I execute the script via the debugger. It still blocks if I startup the script file from the command prompt like node.exe hang.js. I also tried starting up the script on my colleague's computer from the command prompt and got the same result. His computer runs Windows 10.

[BSoD-Ultimate]

Update: I built a debug version of Node.js 10.16.0 (using vcbuild.bat debug) and linked my library against the debug Node.js executable. Then I managed to find out how JavaScript Promise works and which C++ code executes when the JavaScript code calls Promise.resolve. The function JSPromise::Resolve in the file deps/v8/src/objects.cc might be the C++ code relates to the JS method Promise.resolve. I set a breakpoint on this function. When my debug build runs in the interactive shell mode and I invoke my JS script using code like require('hang.js'), all is fine. The program breaks execution at the breakpoint I set before after the console window outputs async task result. When I invoke the script directly from the command line. The problem happens. My debug build did not break execution at the JSPromise::Resolve function(After Nan::Call), it just stuck at running libuv loop and nothing more happens.

[BSoD-Ultimate]

The code is here. https://github.com/BSoD-Ultimate/node_async_await_not_work

[d3x0r]

I have the same issue. I get an event dispatched through uv_async_send(), which invokes callbacks into JS, which result with promises that need to resolve. The uv_async_send is posting to a worker_threads thread. The promise does not resolve, until another message is posted to the thread.

This is the gist of it... https://gist.github.com/d3x0r/2340a660b24f59eae4e58cdfe8fb26e4 (literally and figuratively) .. although I haven't been able to boil it down to a test case, the above c++ stub would be a good start.

I'm sure it doesn't actually require threads, just uv_async_send and a Promise.

I've figured out that processTicksAndRejections which is resonposible for actually dispatching the resolution ( in node/lib/internal/task_queues.js ) somehow isn't getting triggered.

So, how should a C++ addon that dipsatches events trigger the promise resolution task queue?

[addaleax]

@d3x0r It’s hard to tell without code, but a few things:

• The original addon posted by @BSoD-Ultimate uses uv_default_loop(), so it won’t work with worker threads anyway. If you’re doing the same thing, that’s what you want to fix first.
• It also uses Nan::Call() to call its callback function. That doesn’t run the task queues, neither microtask nor nextTick (and misses some other things that should happen when you call from the event loop); you’ll want to use Nan::MakeCallback(), or, even better, Nan::AsyncResource instead.

[d3x0r]

there's lots of code, but it's more than a molehill, so even with code, it wouldn't help a lot.

I back tracked how process._tickCallback gets built to be 'runNextTasks' / 'runMicroTasks' gets called from the JS (and once from the main, after the code is started).

I added process._tickCallback() on the return of onmessage, and promises no longer fail to resolve.

[addaleax]

I added process._tickCallback() on the return of onmessage, and promises no longer fail to resolve.

@d3x0r Yes – please don’t do this. :smile: It’s an internal API and you might get bitten when using it.

That pretty much confirms that the bug is that you’re using Call instead of MakeCallback, though.

[d3x0r]

I don't know... at the moment, it's a little high in the stack... but if the stack is just ...

>	node.exe!wssiAsyncMsg(uv_async_s * handle) Line 734	C++
 	node.exe!uv_process_async_wakeup_req(uv_loop_s * loop, uv_async_s * handle, uv_req_s * req) Line 98	C
 	node.exe!uv_process_reqs(uv_loop_s * loop) Line 195	C
 	node.exe!uv_run(uv_loop_s * loop, uv_run_mode mode) Line 519	C
 	node.exe!node::worker::Worker::Run() Line 299	C++
 	node.exe!node::worker::Worker::StartThread::__l9::<lambda>(void * arg) Line 515	C++
 	node.exe!void <lambda>(void *)::<lambda_invoker_cdecl>(void * arg) Line 515	C++
 	node.exe!uv__thread_start(void * arg) Line 110	C
 	node.exe!invoke_thread_procedure(unsigned int(*)(void *) procedure, void * const context) Line 92	C++
 	node.exe!thread_start<unsigned int (__cdecl*)(void * __ptr64)>(void * const parameter) Line 115	C++
 	[External Code]	

I think that would be a great place to trigger runMicrotasks. Although maybe node::worker::Worker::Run() Line 299

https://github.com/d3x0r/sack.vfs/blob/master/src/websocket_module.cc#L776-L780 (before returning to uv_process_async_wakeup_req

		class constructorSet* c = getConstructors( isolate ); // thread instance data tracker
		Local<Function>cb = Local<Function>::New( isolate, c->ThreadObject_idleProc );
		cb->Call( isolate->GetCurrentContext(), Null( isolate ), 0, NULL );

https://github.com/d3x0r/sack.vfs/blob/master/vfs_module.js#L34 grabs tickCallback on initialization.

Although I'm not sure why the Worker::Run() isn't just doing it when I return.

@addaleax would you mind updating last comment with a link to 'MakeCallback' ?

[addaleax]

Although I'm not sure why the Worker::Run() isn't just doing it when I return.

Because uv_run() doesn’t return control back to Worker::Run() until the thread exits, usually.

would you mind updating last comment with a link to 'MakeCallback' ?

Yeah, I wasn’t sure if you were using Nan or not – for Node, it’s https://github.com/nodejs/node/blob/2205f85b2caadee425a0a86bd8cc3bcb889e4bfe/src/node.h#L821-L851

[d3x0r]

So... promises aren't queued into the loop? It took a long time to figure out why exactly it didn't work when I removed all logging; Logging would generate output from the worker thread to the main thread. I suppose one short path would just be to open a local log file in the thread... but otherwise, generating output would generate thread event messages, which dispatch Promise.resolve( constant ) sort of things. I also noticed if a timer fired, actually in some other thread, that my thread's resolved promises would resolve... And I Thought if I was working with streams, that between streams, it would get handed off to the event loop to be re-dispatched later; but my tall callstack of JS code would say otherwise.

so.... is the real problem that I'm using uv_async_send instead of uv_worker_something ?

Edit: Please to pardon my informatlity, I've been stuck on thie for like a week, thinking 'ahh haha, I figured out how prosies work' and THEN I take out the debugging and it breaks, I cahnge the debugging, and it works.... and then it doesn't. WHy would promsies be ... non-deterministic? I ask on IRC And they laugh at me thinking I don't know how promises work. But it's really that they aren't dispatched.

WHY

a) why can't this just be a documation patch, and/or exposed as a way such that 'if your node addon does ... uv_async_whatever' you should get a deprecation mession... sorry I'm mixing thoughts.

WHY IS THIS A PROBLEM FOR ME?! that I have to do something... that I have to call TICK

[addaleax]

So... promises aren't queued into the loop?

That’s correct – Promises are not tied to the event loop in any way.

.... is the real problem that I'm using uv_async_send instead of uv_worker_something ?

No, the problem really is that you’re using Call() instead of MakeCallback(). uv_async_send() is just fine.

[addaleax]

a) why can't this just be a documation patch, and/or exposed as a way such that 'if your node addon does ... uv_async_whatever' you should get a deprecation mession... sorry I'm mixing thoughts.

Again, that is not the issue here.

WHY IS THIS A PROBLEM FOR ME?! that I have to do something... that I have to call TICK

Please don’t call process._tickCallback(). That’s not what you have to do here.

[d3x0r]

Right....

https://github.com/nodejs/node/blob/master/src/api/callback.cc#L121-L130 (and really 121 and 130)

 if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) return ; // a short circuit I couldn't do myself

Local<Function> tick_callback = env_->tick_callback_function();

Yup, that's the same function. after a bunch of checks would would have already been done , that got me to the state of having my own Scope, instead of a InsternalCallbackScope() (which is the secret sauce of MakeCallback ATM)

I had found this issue https://github.com/nodejs/node/issues/10101 discussing the cost of MakeCallback...

Can't there just be a more direct way for addons to dispatch that? I also wouldn't really want the dispatch to happen with every call, if I have a series of operations pending, there's nothing really to prevent them from also running and creating promises. I'll probably make my implementation either a per-socket option, or well... on demand would be approaching the same overhead.

Please don’t call process._tickCallback(). That’s not what you have to do here.

That's exactly what you're telling me to do; but indirectly. And yes, mine has the cost of maintaining code which will break (and had previously broken) in the future.

[addaleax]

I had found this issue nodejs/node#10101 discussing the cost of MakeCallback...

Keep in mind the age of that issue. A lot of work has gone into optimizing things like process.nextTick(), and the fact that that issue even mentions domain support in MakeCallback() make it seem ancient – domain support is long gone from MakeCallback().

Can't there just be a more direct way for addons to dispatch that?

There are currently three ways of doing this through the Node.js API:

• Using MakeCallback() instead of Call() – it’s not really very different from Call(), tbh.
• Using a CallbackScope – this might be the right thing if you’re worried about batch operations.
• Using the AsyncResource class – this is typically the easiest way of ensuring that your addon works well with async tracking code.

Particularly the last one is somewhat … convenient, and forces addons to properly perform async tracking.

Please don’t call process._tickCallback(). That’s not what you have to do here.

That's exactly what you're telling me to do; but indirectly. And yes, mine has the cost of maintaining code which will break (and had previously broken) in the future.

I mean, yes, that’s true. I can only tell you that there’s a good chance you’ll get burned at some point in the future. (Even though we do try to not break existing code in the wild.)

[addaleax]

Also, if you have ideas for how to make this easier for addons, please feel free to share them, even if you’re not sure whether it’s realistic or not.

[d3x0r]

I understand they're not realistic; /you don't want/it's not good to have/ people willy-nilly calling the tick callback. (like in a JS callback for instance, and worse, in a JS callback that's also in a callback through native code... eventually it's all bad anyway; and one re-engineers the solution).

After some consideration, I would probably implement the callback scheduler to post an event to run, and if it's already posted, don't re-post, but then, just fall back on the uv_run dispatcher, with its next task being micro-task-things...

Edit: I said before; I had tried to use promsies and they just failed to work; I didn't even consider then that it might be because it was an event from websocket dispatched through uv_async_send... so of course any other work that got caused must be in line after me... so much time wasted for something so simple to have implemented....

---

But let me come back to why I clicked reply - and maybe it's just being snarky...

Let me first search on https://nodejs.org/dist/latest-v13.x/docs/api/addons.html for 'MakeCallback' 0 mentions.

Let me search site:nodejs.org MakeCallback

https://nodejs.org/api/all.html

• DEP0099: async context-unaware node::MakeCallback C++ APIs
• DEP0097: MakeCallback with domain property
• a few others, no examples

https://nodejs.org/api/deprecations.html

Hmm... how am I, as a developer, supposed to know that there's this magic MakeCallback() that dispatches a timer event to appease promises?

I've been working with Node since late 6 and 7 beginning.... It's(MakeCallback) STILL not in the node addon page; and it certainly wasn't then.

I've been trying for so very long to get promises to work, and it turns out to just be that uv_async_send doesn't itself cause promise resolution; but must be handled, somehow, in some way which is unknown, and unmentioned.

[d3x0r]

I realized, I mean to say, you're just asking me to use a different undocumented API....

[BSoD-Ultimate]

I'd like to share my experiences in getting rid of this problem at that time.(Sorry for not updating the issue on time!) Later the time I posted this issue, I found out one ugly way which solves my issue.

My approach is setting a timer signaling SIGWINCH in a certain period of time. The method was found by noticing codes in the async callback get executed when I modified the console window's size by chance. Inspecting C++ source codes related to the signal handler in Node.js also lead me to the mechanism of TickCallback in the C++ way, which is mentioned by @d3x0r.

Following instructions using Nan::AsyncResource instead of directly call Nan::Call in the C++ code as I saw updates on this issue, the problem got solved. This is also mentioned in documentations for Nan saying We recommend that you use the AsyncResource class and AsyncResource::runInAsyncScope instead of using Nan::MakeCallback or v8::Function#Call() directly.

I was lead to a wrong way by outdated examples from Google and the documentation of Node.js at old times, which emphasizes MakeCallback. The async context in Node.js really does matter. Anyway, thank you for all.

PS: By following the Nan::AsyncResource way, in the C++ code I given in the issue, I added a Nan::AsyncResource-typed member in the class Worker. The direct calling of Nan::Call was replaced to the Nan::runInAsyncScope method in the following form:

rs.runInAsyncScope(Nan::New<v8::Object>(), callback->GetFunction(), argc, argv.get());

I used the test case https://github.com/nodejs/nan/blob/master/test/cpp/asyncresource.cpp as the reference.

[d3x0r]

Am not sure this should be closed... would hope others would find it in the future (although, they really shouldn't have to)... you can unsubscribe? I wonder how many thousands of other projects hacked their way through it with like a setImmediate or some other dependancy package because promises don't post to the uv_run loop like everything else; and special interfacing with node is required.

[sergivillar]

Hi @BSoD-Ultimate, I'm struggling with the same issue as you , could you please update your example repository with the fix please?

thanks

[addaleax]

@d3x0r I haven’t forgotten about this issue, and I’ll happily re-open :) There’s definitely at least a major deficiency in our addons documentation.

And, fwiw, I had that same issue in the past and worked around it with setImmediate() because I didn’t know when to use MakeCallback.

[d3x0r]

@addaleax Thank you for the affirmation I'm not just being a noob. setImmediate would do it too; could just wrap the dispatch all in that :)

I still think there's no reason promises should be so special that they don't go to the run loop too. But certainly since v0.11.13 answered Mar 13 '14 at 20:55 noone else thinks this is an issue that should be resolved this way; afterall, just use makeCallback; when you first mentioned that, I asked to a link of an example, the prototype is not itself a very good example; why do I need to pass a string name (for instance). And the name of MakeCallback is misleading since it kinda look more like it's a DoCallback.

G'day. It all works today...

[BSoD-Ultimate]

@sergivillar The repository has been updated for a solution that might solve this issue. The key point is to always use Nan::AsyncResource::runInAsyncScope instead of directly calling Nan::Call when you have to notify the JavaScript code when an async operation completes as the documentation says.

[addaleax]

I still think there's no reason promises should be so special that they don't go to the run loop too.

@d3x0r I’m not sure what exactly you refer to here, but just to clarify:

• Promises aren’t tasks, they represent the results of tasks
• The way the JS spec defines Promise operations, I don’t think we could put things like .then() or async/await on the event loop like setImmediate() does; they would keep having to behave like process.nextTick()

[d3x0r]

I still think there's no reason promises should be so special that they don't go to the run loop too.

@d3x0r I’m not sure what exactly you refer to here, but just to clarify:

• Promises aren’t tasks, they represent the results of tasks I understand that perspective; however, the resolution of promises is itself a task. (a series of things that need to be executed).

• The way the JS spec defines Promise operations, I don’t think we could put things like .then() or async/await on the event loop like setImmediate() does; they would keep having to behave like process.nextTick() An addition event post (uv_async_send) to trigger constant promise resolution that hasn't otherwise been triggered would not require them to be any such thing. In many cases there is another I/O event that will come in, and result in some code that invokes nextTick() in some way... ( like setImmediate/timeout/... or like posting a message to process.stdin or communicating with workerThread.post() and resulting onMessage ... all these builtin these must all have some hook to result in a nextTick() invokation.

And mind you there was one little thing in there... 'constant/statically resolved promise' ... In my case the API is a bunch of getters from a worker to the main... The workers cache the result, because any later change will be posted by the main to the work. The first transaction, the thread posts a message, and reuslts in a promise which will be fulfilled by IO, and gets a response later, which resolves the promise, and (maybe? ) resolves that microtask at that time, and has nothing to do with going through nextTick (I'm not sure it matters); But, on the second request, the result is a Promise.resolve( existingValue ); ... (obviously) this cannot actually resolve yet, because any .thens and catches added later on the return path haven't been assigned yet... (and apparently when they do get assigned, with an already fulfilled promise, they don't just fire) but instead at the end a promise chain exists with a resolution already when JS returns to the native code that invoked it (and which subsequently returns to the work loop). Apparently, Promise.resolve() (or a return of a constant from an async function) should additionally have a hook to post a uv_async_send() event (unless it's already been posted, and the JS, being single threaded can easily keep a JS level flag tracking this, which clears the 'has been posted' when the event actually posts from the uv loop; at which point it either has 0 work to do, (because some other thing along the way was able to run microtasks before the event loop got here) or has some static promise resolution dispatch to do.

Promises do also lead into being tasks...

(I'm somewhat just rambling here...) Just somewhere someone patched it in in some way that a bunch of code inherited being able to trigger - dispatch any remaining events. ... when the uv_async_events get run, the JS system is 100% idle... it's like calling a sched_yield(). Certainly it shouldn't be a huge impact to fall back to the uv_run loop and catch a one process.nextTick() invokation. (unless that promise resolution results in more Promise.resolve()ed chains, but maybe because it's In the microtask dispatch, maybe Those ... no... would still have to wait for the call stack to return basically back to nothing.)

[gireeshpunathil]

@BSoD-Ultimate - is this still an issue?

[d3x0r]

It is and isn't; if you 1) submit to using NaN or 2) call next tick yourself that the NaN libraries does automagically for you https://github.com/nodejs/help/issues/2028#issuecomment-560042063 Since only 1) is supported, and they refuse to let anyone not using NaN to use Promises I guess it's not an issue (since I wasn't left with a proper way to call the function, just a lot of 'don't do what you need to do to make it work'.

[github-actions[bot]]

It seems there has been no activity on this issue for a while, and it is being closed in 30 days. If you believe this issue should remain open, please leave a comment. If you need further assistance or have questions, you can also search for similar issues on Stack Overflow. Make sure to look at the README file for the most updated links.