Repository navigation
n-api: regression with napi_delete_async_work since v10.2.0 #20966
Description
- Version: v10.2.1
- Platform: any
- Subsystem: n-api
I'm experimenting with n-api in a branch of iltorb: nstepien/iltorb#70
I added calls to napi_delete_async_work after forgetting to make use of it, but I ended up getting rogue segmentation faults.
The tests pass on previous versions of Node on various CI, and also on Node v10.1.0 locally, but not Node v10.2.0.
Removing the calls to napi_delete_async_work seems to appease Node v10.2.x.
Activity
hashseed commentedon May 31, 2018
@mhdawson @addaleax @aruneshchandra
gabrielschulhof commentedon Jun 4, 2018
@MayhemYDG can you create a minimum test case to illustrate the problem?
nstepien commentedon Jun 4, 2018
@gabrielschulhof I'm trying to create a minimum test case from scratch, but it's not failing unfortunately...
There must be more to it but I don't know where to look. It only happens on Node 10.2.0+.
gabrielschulhof commentedon Jun 4, 2018
gabrielschulhof commentedon Jun 4, 2018
@MayhemYDG https://gist.github.com/gabrielschulhof/4d01d88a75df33981511304162c4a063 crashes when built against master, but runs fine prior to commit c072057.
gabrielschulhof commentedon Jun 4, 2018
This is a bug we've fixed before. Before calling the
_completecallback we must back up theenv, because the copy stored inside the wrapper class gets lost when the wrapper class gets freed as part ofnapi_delete_async_work().gabrielschulhof commentedon Jun 4, 2018
Actually, we've fixed it before, but not with async work: 1a5a19d 🙂
n-api: back up env before async work finalize
s-h-a-d-o-w commentedon Jun 29, 2018
Are you sure this fix resolved everything? Because my simple test (https://github.com/s-h-a-d-o-w/napi-test) still crashes at the call to
napi_delete_async_work- using v10.5.0.gabrielschulhof commentedon Jun 30, 2018
@s-h-a-d-o-w I'll try to reproduce the crash.
gabrielschulhof commentedon Jun 30, 2018
@s-h-a-d-o-w at https://github.com/s-h-a-d-o-w/napi-test/blob/master/src/napi_init.cc#L103-L106 you assign
workintodata->workbefore passingworkintonapi_create_async_work()so that it might become initialized.If you move
data->work = workafternapi_create_async_work()the segfault will be resolved.s-h-a-d-o-w commentedon Jun 30, 2018
Ahhh! Thanks!
n-api: back up env before async work finalize
n-api: back up env before async work finalize