Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Document that the ObjectWrap example code doesn't work for multi-context usage #711

Closed
geekuillaume opened this issue Apr 27, 2020 · 2 comments
Closed

Document that the ObjectWrap example code doesn't work for multi-context usage #711

geekuillaume opened this issue Apr 27, 2020 · 2 comments

Comments

@geekuillaume
Copy link

After spending a day trying to debug why I had so many seemingly random issues with my code, I finally understood why: the object wrap example code is not compatible with a multi-context usage of the addon.

Explanation: this example set the constructor static member of the class to a reference of the constructor function but if the addon is loaded multiple time (with worker_threads for example), the value is overwritten by a new reference in the new context. That makes this reference unusable to create new instances of this wrapped class from other C++ code and makes any code wanting to use this reference in another context crash.

We should at least make sure there is a warning about this in the documentation. The best would be to provide a workaround with the new addon instance data support maybe.
@mhdawson
Copy link
Member

mhdawson commented May 6, 2020

@geekuillaume any chance you'd like to submit a PR to the docs to cover this?

@markdirish
Copy link

@geekuillaume what kind of errors were you seeing?

I am seeing some errors with HandleScopes in an AsyncWorker's OnOk() when using worker_threads. Looking at similar issues across other repos, it seems the issue is 'context-aware addons', but being a node-addon-api addon (and therefore a N-API addon) I don't think I'm doing anything wrong as far as declaring the module.

I'm not sure if this is the same issue you are running into, but seems possible.

@necil necil mentioned this issue May 13, 2020
Closed
@nick-erm nick-erm mentioned this issue May 27, 2020
Open
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jun 29, 2020
doc: update ObjectWrap example
413134c 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs#711
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jun 29, 2020
doc: update ObjectWrap example
28e53d1 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs#711
@gabrielschulhof gabrielschulhof mentioned this issue Jun 29, 2020
Closed
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jun 29, 2020
doc: update ObjectWrap example
6252e48 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs#711
gabrielschulhof pushed a commit to gabrielschulhof/node-addon-api that referenced this issue Jul 1, 2020
doc: update ObjectWrap example
e095c01 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs#711
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this issue Aug 24, 2022
@kevindavies8
doc: update ObjectWrap example
5228631 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this issue Aug 26, 2022
@Marlyfleitas
doc: update ObjectWrap example
e399318 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this issue Sep 19, 2022
@wroy7860
doc: update ObjectWrap example
9f013be 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this issue Aug 11, 2023
doc: update ObjectWrap example
b6a87f5 
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@uurien uurien mentioned this issue Jan 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

doc: update ObjectWrap example gabrielschulhof/node-addon-api
3 participants
@geekuillaume @mhdawson @markdirish