fix: empty data OnProgress by race conditions of AsyncProgressWorker #853
Conversation
Yeah, I've identified the problem. There might be an exception leaking in threadsafe function callbacks in node core so that the promise chains of the node-addon-api are broke up yet the thrown JavaScript exception is not handled properly and the process exits silently. I'll try to file a minimum reproduce to get that fixed in node core. Besides that, I'll update the PR to get tests running. |
eb7b4e2 to
e7cdc8e
Compare
|
@mhdawson |
Just filed WIP nodejs/node#36510. Need to figure out if there are any other cases that also swallowing exceptions. |
|
@legendecas thanks for figuring out the issue and fixing up the tests. They all pass for me, landing. |
|
Landed as 86feeeb |
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes: nodejs/node-addon-api#821 PR-URL: nodejs/node-addon-api#853 Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
Fixes #821
The callback of ThreadSafeFunction is not been invoked immediately on the callback of uv_async_t (uv io poll), rather the callback of TSFN is invoked on the right next uv idle callback. There are chances that during the deferring the signal of uv_async_t is been sent again, i.e. potential not coalesced two calls of the TSFN callback.
However, when the race condition happens, the AsyncProgressWorker::OnProgress is likely not been called yet. There is hardly a fix on ensuring the race condition not happening.
In summary, the fix is suppressing the unexpected duplicated calls of AsyncProgressWorker::OnProgress.