Context-sensitive examples are not extensible

オープン
  • 何か問題がありますか？
#171 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
35/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
停滞
技術スタック
cpp, nodejs
領域
api, developer-experience

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
C++
スター
2.6k
フォーク
598
30日以内にマージされた PR はありません

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

nodejs/node-addon-examples のほかの issue

nodejs/node-addon-examples の issue をすべて見る

似ている issue

C++ の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。