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

Context-sensitive examples are not extensible #171

Open
ikokostya opened this issue Jan 16, 2021 · 0 comments
Open

Context-sensitive examples are not extensible #171

ikokostya opened this issue Jan 16, 2021 · 0 comments

Comments

@ikokostya
Copy link

This pull request #139 removes global static references from the examples. However, proposed solution works only for single object wrap.

Consider the following example where two object wraps are created using the same code snippet:

Napi::FunctionReference* constructor = new Napi::FunctionReference();
*constructor = Napi::Persistent(func);
env.SetInstanceData(constructor);

test.cc

#include <napi.h>

#include <cassert>

class Foo : public Napi::ObjectWrap<Foo> {
 public:
  static void Init(Napi::Env env, Napi::Object exports) {
    Napi::Function func = DefineClass(env, "Foo", {});

    Napi::FunctionReference* constructor = new Napi::FunctionReference();
    *constructor = Napi::Persistent(func);
    env.SetInstanceData(constructor);

    exports.Set("Foo", func);
  }

  static Napi::Object NewInstance(Napi::Env env) {
    const auto constructor = env.GetInstanceData<Napi::FunctionReference>();
    assert(constructor != nullptr);
    return constructor->New({});
  }

  explicit Foo(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Foo>{info} {}
};

class Bar : public Napi::ObjectWrap<Foo> {
 public:
  static void Init(Napi::Env env, Napi::Object exports) {
    Napi::Function func = DefineClass(env, "Bar", {});

    Napi::FunctionReference* constructor = new Napi::FunctionReference();
    *constructor = Napi::Persistent(func);
    env.SetInstanceData(constructor);

    exports.Set("Bar", func);
  }

  static Napi::Object NewInstance(Napi::Env env) {
    const auto constructor = env.GetInstanceData<Napi::FunctionReference>();
    assert(constructor != nullptr);
    return constructor->New({});
  }

  explicit Bar(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Foo>{info} {}
};

Napi::Value CreateFoo(const Napi::CallbackInfo& info) {
  return Foo::NewInstance(info.Env());
}

Napi::Value CreateBar(const Napi::CallbackInfo& info) {
  return Bar::NewInstance(info.Env());
}

Napi::Object InitAll(Napi::Env env, Napi::Object exports) {
  Foo::Init(env, exports);
  Bar::Init(env, exports);
  exports.Set("createFoo", Napi::Function::New(env, CreateFoo));
  exports.Set("createBar", Napi::Function::New(env, CreateBar));
  return exports;
}

NODE_API_MODULE(NODE_GYP_MODULE_NAME, InitAll)

test.js

'use strict';

const assert = require('assert');
const {createFoo, createBar, Foo, Bar} = require('./build/Debug/test');

const foo = createFoo();
assert(foo instanceof Foo); // fails
const bar = createBar();
assert(bar instanceof Bar);
$ node test.js
assert.js:383
    throw err;
    ^

AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:

  assert(foo instanceof Foo)

    at Object.<anonymous> (/home/kostya/tmp/napi-test/test.js:7:1)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

As we can see second call of Env::SetInstanceData() in Bar::Init() overrides reference to Foo constructor. Moreover, this causes memory leak, because destructor of the first reference will not be called

From https://nodejs.org/dist/latest-v14.x/docs/api/n-api.html#n_api_napi_set_instance_data

Any existing data associated with the currently running Agent which was set by means of a previous call to napi_set_instance_data() will be overwritten. If a finalize_cb was provided by the previous call, it will not be called.

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

No branches or pull requests

1 participant