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.

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

Example Object Wrapping code never calls destructor #19876

Closed
wyckster opened this issue Apr 8, 2018 · 3 comments
Closed

Example Object Wrapping code never calls destructor #19876

wyckster opened this issue Apr 8, 2018 · 3 comments
Labels
doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.

Comments

@wyckster
Copy link

wyckster commented Apr 8, 2018

  • Version: v9.11.1
  • Platform: Windows 10 Enterprise 64-bit
  • Subsystem: documentation

The documentation for Wrapping C++ Objects has an example that creates a class called MyObject which derives from node::ObjectWrap.

It would be very helpful if the documentation described the mechanism by which the object will be deleted (how/when) and provided an example of how to get the destructor to actually be invoked in practice, since it will only get deleted when the garbage collector runs. As I understand it (as I am able to demonstrate by running the example), the destructor will not get called. The example suggests that one is intended to run the following code, which will not induce a garbage collection:

// test.js
const addon = require('./build/Release/addon');

const obj = new addon.MyObject(10);
console.log(obj.plusOne());
// Prints: 11
console.log(obj.plusOne());
// Prints: 12
console.log(obj.plusOne());
// Prints: 13

It would be much better if there were an example that shows how to force a garbage collection that causes the wrapped object to get properly destroyed so that developers of addons can ensure that C++ resources are not leaked.

As I see it, this is only half an example because it only demonstrates creating objects, not destroying them. They are not demonstrated to get destroyed using this example code. I think this is a shortcoming of the documentation.

@joyeecheung joyeecheung added the doc Issues and PRs related to the documentations. label Apr 8, 2018
@joyeecheung
Copy link
Member

If anyone wants to pick this one up, there is a similar example for napi in the tests:
https://github.com/nodejs/node/blob/master/test/addons-napi/8_passing_wrapped

@apapirovski apapirovski added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Apr 11, 2018
@isurusiri
Copy link
Contributor

If no one else is working on this, I'd like to give this a try.

Trott pushed a commit to Trott/io.js that referenced this issue Sep 19, 2018
Currently the documentation for Wrapping C++ Objects doesn't explain
how to destruct an object by explicitly invoking the garbage collector.
This commit includes a modification to docs that explains how to force
the garbage collector to clear objects using V8's command line flags.

Fixes: nodejs#19876

PR-URL: nodejs#20431
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
@Trott
Copy link
Member

Trott commented Sep 19, 2018

Fixed in a1381fa

@Trott Trott closed this as completed Sep 19, 2018
targos pushed a commit that referenced this issue Sep 19, 2018
Currently the documentation for Wrapping C++ Objects doesn't explain
how to destruct an object by explicitly invoking the garbage collector.
This commit includes a modification to docs that explains how to force
the garbage collector to clear objects using V8's command line flags.

Fixes: #19876

PR-URL: #20431
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
targos pushed a commit that referenced this issue Sep 20, 2018
Currently the documentation for Wrapping C++ Objects doesn't explain
how to destruct an object by explicitly invoking the garbage collector.
This commit includes a modification to docs that explains how to force
the garbage collector to clear objects using V8's command line flags.

Fixes: #19876

PR-URL: #20431
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. help wanted Issues that need assistance from volunteers or PRs that need help to proceed.
Projects
None yet
Development

No branches or pull requests

5 participants