n-api: regression with napi_delete_async_work since v10.2.0

[nstepien]

• 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.

[hashseed]

@mhdawson @addaleax @aruneshchandra

[gabrielschulhof]

@MayhemYDG can you create a minimum test case to illustrate the problem?

[nstepien]

@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]

I think I've reproduced it.

…

On Mon, Jun 4, 2018 at 3:29 PM, Nicolas Stepien ***@***.***> wrote: @gabrielschulhof <https://github.com/gabrielschulhof> I'm trying to create a minimum test case from scratch, but it's not failing... There must be more to it but I don't know where to look. It only happens on Node 10.2.0+. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20966 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA7k0RY5TBE7pP-aXNvLegEQFUgTzPf_ks5t5YqRgaJpZM4UOZsE> .

[gabrielschulhof]

@MayhemYDG https://gist.github.com/gabrielschulhof/4d01d88a75df33981511304162c4a063 crashes when built against master, but runs fine prior to commit c072057.

[gabrielschulhof]

This is a bug we've fixed before. Before calling the _complete callback we must back up the env, because the copy stored inside the wrapper class gets lost when the wrapper class gets freed as part of napi_delete_async_work().

[gabrielschulhof]

Actually, we've fixed it before, but not with async work: 1a5a19d 🙂