Context-sensitive examples are not extensible

Ouverte
  • Quelque chose ne va pas ?
#171 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Évaluation

Difficulté
4/5
Temps estimé
3-5 jours
Accessibilité débutants
35/100
Type d'issue
Bug
Clarté
Plutôt claire
Activité
À l'abandon
Stack technique
cpp, nodejs
Domaine
api, developer-experience

Piste de recherche

Start with the test.cc and test.js reproducer, then read the N-API SetInstanceData documentation cited in the issue. Verify how creating Foo and Bar causes the second instance-data value to replace the first. Done means both createFoo() and createBar() produce objects passing their respective instanceof checks without leaking the earlier constructor reference.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Description

This pull request https://github.com/nodejs/node-addon-examples/pull/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.

Langage dominant
C++
Étoiles
2.6k
Forks
598
Aucune PR mergée en 30 j

Préparer son environnement

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Autres issues de nodejs/node-addon-examples

Toutes les issues de nodejs/node-addon-examples

Issues similaires

Plus d'issues C++

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.