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

Memory leak in 1-getting-started/6_object_wrap ? #580

Open
graebm opened this issue Jan 17, 2025 · 1 comment
Open

Memory leak in 1-getting-started/6_object_wrap ? #580

graebm opened this issue Jan 17, 2025 · 1 comment

Comments

@graebm
Copy link

graebm commented Jan 17, 2025

MyObject* obj = new MyObject(value);
obj->env_ = env;
status = napi_wrap(env,
jsthis,
reinterpret_cast<void*>(obj),
MyObject::Destructor,
nullptr, // finalize_hint
&obj->wrapper_);

The code above allocates memory for MyObject via new (which allocates and calls MyObject::MyObject()), but I don't see where that memory is ever freed?

Here's the finalizer...

void MyObject::Destructor(napi_env env,
void* nativeObject,
void* /*finalize_hint*/) {
reinterpret_cast<MyObject*>(nativeObject)->~MyObject();
}

I believe that finalizer code should be like:

  MyObject* obj = reinterpret_cast<MyObject*>(nativeObject);
  delete obj;

delete will call MyObject::~MyObject() AND free the memory

@mhdawson
Copy link
Member

@graebm, we discussed in the node-api team meeting today and agree the example should use delete instead of calling ~MyObject(). Would you like to submit a PR to fix that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Need Triage
Development

No branches or pull requests

2 participants