test: Add tests for escape and handlescope

[JckXia]

Adding test coverage for Escape and HandleScope, closes #990

[mhdawson]

Could you have used a second HandleScope declaration here instead of making the node-api call directly? I think it would then look like

HandleScope scope_one(info.Env())
HandleScope scope_two(info.Env(), scope_one.napi_handle_scope())

Same would go for the other test added as well.

[JckXia]

I think I've tried that approach before, but ran into some issues with napi_close_handle_scope in ~HandleScope being called on the same underly napi_handle_scope twice when they go out of scope.

[mhdawson]

@JckXia if that's the case then I think we might have a real problem?

[JckXia]

Hmm, I believe the HandleScopeconstructor where we provide an existing napi_handle_scope simply saves a reference of the napi_handle_scope passed in, as opposed to something like a move constructor where we set the other to null.

I think this means when scope_two goes out of scope, its dtor gets ran, we successfully close the referenced handle scope. But when scope_one goes out of scope, its dtor will attempt to call napi_close_handle_scope on what I assume is an already closed handle scope, resulting in a napi_handle_scope_mismatch error.

The reason I created the napi_handle_scope via a direct call to node-api is such that only scope_two's dtor will be ran, closing it successfully.

[JckXia]

Sorry @mhdawson , I think I should've been more specific.

The HandleScope::HandleScope(napi_env env, napi_handle_scope scope) overload that we are testing here simply takes an existing napi_handle_scope ref and constructs a C++ stack object. I believe that ctor overload does not open any new scopes in V8.

This napi_handle_scope ref was created by HandleScope scope_one(napi_env), via the HandleScope::HandleScope(napi_env env) ctor from the example. In other words, now we have two HandleScope objects on the C++ stack that is referencing the same napi_handle_scope pointer.

When we reach the end of the function, scope_two goes out of the C++ stack, and its dtor gets ran. In HandleScope::~HandleScope we make a call to napi_close_handle_scope to reflect this change on V8's side. Its successfully ran, and now from V8's POV we have 0 open_handle_scopes. Then, scope_one goes out of the C++ stack, and we run its dtor. We make another call to napi_close_handle_scope, but by now there isn't any open handle scopes in env, (https://github1s.com/nodejs/node/blob/HEAD/src/js_native_api_v8.cc#L2535), which is why we see the napi_handle_scope_mismatch_error

[mhdawson]

I looked a little more carefully and I understand now. It's not that we have two scopes where one is beeing used in the second. Instead the API is to create an node-addon-api object from an existing node-api C handle. In that case creating the handle through a node-addon-api object would result in a double free since there would be two node-addon-api objects pointing to the same node-api C handle.

Based on that I think this looks good :)

[mhdawson]

LGTM

[mhdawson]

Landed as 35d9d66

[JckXia]

Thanks! @mhdawson :)