Node-addon-api: Context-aware add-ons?

Created on 16 Oct 2019  Ā·  20Comments  Ā·  Source: nodejs/node-addon-api

I am reading this https://nodejs.org/api/addons.html#addons_context_aware_addons and am left wondering if node-addon-api can help making context aware native modules.

There does not seem to be a way to initialise a context-aware module with node-addon-api currently.

Additionally it seems to me that it would be nice to provide a default wrapper for per-context global state (what would normally be easily accomplished with static variables).

Any thoughts?

stale

All 20 comments

Fwiw, all N-API addons are considered context-aware.

Additionally it seems to me that it would be nice to provide a default wrapper for per-context global state (what would normally be easily accomplished with static variables).

I think napi_set_instance_data might be what you want – it’s technically behind the NAPI_EXPERIMENTAL flag, but you can use it if you’re targeting the latest version of Node.js.

(The documentation for it is kind of misleading imo – Node.js currently does not support loading addons from multiple vm Contexts, so right now it’s not wrong to say that the data is associated with the current agent, but conceptually, the association is with the current module instantiation, which would be per-Context and not per-Agent.)

It does seem like napi_set_instance_data/napi_get_instance_data will provide a good solution. So all that is required is a nice pattern to initialise a module, associate a C++ class with the instance data and support retrieving it from a given Napi::Env handle.

Is there any way to polyfill this for Node 10? Would be a shame to have to wait for Node 10 to be out of LTS for it to be generally usable for add-ons.

@gabrielschulhof I think you implemented napi_set_instance_date/napi_get_instance_data. I also remember that we refactored the internal structure at some point. Do you remember if that was before/after 10.x as that might affect how easily those two functions can be supported in the 10.x line.

@rolftimmermans there are several solutions to attaching data to an addon instance, in increasing order of correctness and convenience, but decreasing availability in terms of older Node.js versions:

  • Use napi_wrap() to attach data to exports to find out when to delete the per-instance data, and pass the data pointer to each binding created with Napi::Function::New(), which has a parameter void* data.
    This can be done with the earliest version of Node.js, but will not actually free the data until the version that ships with https://github.com/nodejs/node/pull/28428, because exports is never actually garbage-collected.
  • Use napi_add_env_cleanup_hook() and pass the data pointer to each binding created with Napi::Function::New(). This is also available down to v8.12.0, and will actually be called when the environment exits.
  • Use napi_set_instance_data() and napi_get_instance_data(). This is most convenient because you need not pass a void* data to each function you create, but this is only available as of v12.8.0.

I agree that we could use a nice abstraction on top of these mechanisms to render addons as instance factories.

whew ... I did several edits to my comment after I wrote it. Please do not read the emailed version šŸ˜€

Thanks for clearing this up! Very helpful. I see that ObjectWrap<T>::DefineClass also takes a void* argument which means that together with Function::New it appears to solve this problem.

So it turns out that napi_add_env_cleanup_hook() works really well for this use case, but there are more issues that are solved by https://github.com/nodejs/node/pull/28428. Would be really nice if this fix can make it into Node 12 LTS.

I have gone down the napi_add_env_cleanup_hook() route, but am now figuring out that it seems impossible to detect if the env that is being cleaned up is the main thread (at process exit) or a worker thread. Apart from some thread/agent-specific state, the module I'm working on also has some shared, thread-safe, truly global state that I want to tear down at process exit.

I guess It's just not possible right now to detect when the module is loaded/released from the main thread, correct?

I have attempted to use static destructors for this cleanup but it does not seem to work correctly on windows.

@addaleax how safe is it to assume that the relationship between the environment and the thread is 1:1?

@rolftimmermans could you use thread-safe refcounting to decide when to tear down the per-process state? I'm fairly certain that the main thread is the last one to exit.

You can have multiple napi environments in a single thread, but you can't have multiple threads interacting with a napi environment.

@addaleax @devsnek In my previous question I was thinking about Node::Environments, actually, not napi_envs. Sorry about not being explicit!

Alright, the following is a summary of I have now, based on your comments. It might be useful for others.

This seems to work well for Node 10+, except that it appears https://github.com/nodejs/node/pull/28428 won't be included in a stable version until Node 14, which means that so far any use of threads with native addons is giant memory leak (I haven't found a reliable workaround for this unfortunately – suggestions welcome!).

class GlobalState {
    // ... Globally shared, thread-safe state.
    // e.g. uint64_t FooBar;

public:
    static std::shared_ptr<GlobalState> Instance() {
        static std::mutex lock;
        static std::weak_ptr<GlobalState> shared;

        std::lock_guard<std::mutex> guard(lock);

        // Get an existing instance from the weak reference, if possible.
        if (auto instance = shared.lock()) {
            return instance;
        }

        // Create a new instance and keep a weak reference.
        // Global state will be cleaned up when last thread exits.
        auto instance = std::make_shared<ContextReaper>();
        shared = instance;
        return instance;
    }
};

class Module {
    // Pointer to shared global state
    std::shared_ptr<GlobalState> global_state = GlobalState::Instance();

    // ... Other per-module state
    // e.g.: Napi::FunctionReference foo_constructor;

public:
    GlobalState& GlobalState() {
        return *global_state;
    }

    Module(Napi::Object exports) {
        // Initialization per module, assigns constructors, etc.
        auto proto = {
            // ... InstanceMethod("foo", &Foo::foo),
        };

        // Pass module pointer as last argument.
        DefineClass(exports.Env(), "Foo", proto, this);
    }
};

class Foo : public Napi::ObjectWrap<Foo> {
    Module& module;

public:
    // Construct native object. `info.Data()` contains pointer to module.
    Foo(const Napi::CallbackInfo& info)
    : Napi::ObjectWrap<Foo>(info),
      module(*reinterpret_cast<Module*>(info.Data())) {
        // Access global state with e.g. `module.GlobalState().FooBar`
    }
}

NAPI_MODULE_INIT(/* env, exports */) {
    auto module = new Module(Napi::Object(env, exports));
    auto terminate = [](void* data) { delete reinterpret_cast<Module*>(data); };

    auto status = napi_add_env_cleanup_hook(env, terminate, module);
    assert(status == napi_ok);
    return exports;
}

In the future the module-wide state could be accessed via Napi::Environment if it wraps napi_set_instance_data/napi_get_instance_data, and that means the pointer to the module does not have to be passed during class definition, correct?

@addaleax how safe is it to assume that the relationship between the environment and the thread is 1:1?

@gabrielschulhof This is currently always true for the Node.js release binary/when used with Workers, but native addons or embedders don’t need to follow any rules regarding relationships between threads and Environment instances. I would avoid relying on any relationship of that kind as far as possible.

Apart from some thread/agent-specific state, the module I'm working on also has some shared, thread-safe, truly global state that I want to tear down at process exit.

@rolftimmermans If you are looking for ā€œglobalā€ state that should be torn down when your addon is unloaded, then static data in your addon code should be the way to go.

This seems to work well for Node 10+, except that it appears nodejs/node#28428 won't be included in a stable version until Node 14

Why do you think it won’t be included into Node 13 and backported to earlier release lines? While I certainly think we should be cautious about that, I think this is what we should be doing.

which means that so far any use of threads with native addons is giant memory leak (I haven't found a reliable workaround for this unfortunately – suggestions welcome!).

Yeah, that’s just a really, really unfortunate fact in terms of N-API’s design. You should be able to use napi_add_env_cleanup_hook in order to emulate the behaviour introduced in nodejs/node#28428, but that’s going to be a lot of manual work unless node-addon-api implements it for all references on its own.

Why do you think it won’t be included into Node 13 and backported to earlier release lines? While I certainly think we should be cautious about that, I think this is what we should be doing.

@addaleax It would be nice if it could be back-ported, definitely!

Yeah, that’s just a really, really unfortunate fact in terms of N-API’s design. You should be able to use napi_add_env_cleanup_hook in order to emulate the behaviour introduced in nodejs/node#28428, but that’s going to be a lot of manual work unless node-addon-api implements it for all references on its own.

Having it in node-addon-api would be ideal. Maybe with a compile time flag if it has any runtime overhead. If I have time, I might try to work on an implementation. Any tips how to approach this would be welcome!

@rolftimmermans I have a POC that does not use node-addon-api, but which can no doubt be adapted:

The boilerplate:

template <class AddonClass>
class AddonBase {
 public:
  virtual ~AddonBase() {
    while(methods != nullptr) {
      MethodData* next = methods->next;
      delete methods;
      methods = next;
    }
  }

  virtual napi_value Init(napi_env env, napi_value exports) {
    return exports;
  }

  static napi_value New(napi_env env, napi_value exports) {
    AddonClass* instance = new AddonClass();
    napi_set_instance_data(env,
                           instance,
                           Delete,
                           nullptr);
    return instance->Init(env, exports);
  }

  static void Delete(napi_env env, void* data, void* hint) {
    delete static_cast<AddonClass*>(data);
  }

  typedef napi_value(AddonClass::*MethodPointer)(napi_env env,
                                                 napi_callback_info info);

  struct MethodData {
   public:
    MethodData(MethodPointer cb, AddonClass* instance, MethodData* next):
      cb(cb), instance(instance), next(next) {}
    MethodPointer cb;
    AddonClass* instance;
    MethodData* next;
  };

  void AddMethod(napi_env env,
                 napi_value exports,
                 const char* name,
                 MethodPointer method) {
    napi_value result;
    methods = new MethodData(method, static_cast<AddonClass*>(this), methods);
    napi_create_function(env,
                         name,
                         NAPI_AUTO_LENGTH,
                         Dispatcher,
                         methods,
                         &result);
    napi_set_named_property(env, exports, name, result);
  }

 private:
  static napi_value Dispatcher(napi_env env, napi_callback_info info) {
    size_t argc = 0;
    void* raw_data;
    napi_get_cb_info(env, info, &argc, nullptr, nullptr, &raw_data);
    MethodData* mdata = static_cast<MethodData*>(raw_data);
    return ((mdata->instance)->*(mdata->cb))(env, info);
  }
  MethodData* methods = nullptr;
};

and its usage:

class MyAddon: public AddonBase<MyAddon> {
 public:
  napi_value Init(napi_env env, napi_value exports) override {
    // List methods to expose here.
    AddMethod(env, exports, "somethingUseful", &MyAddon::SomethingUseful);
    return exports;
  }

  // The binding is an instance method and so has access to the instance.
  napi_value SomethingUseful(napi_env env, napi_callback_info info) {
    napi_value result;
    napi_create_uint32(env, ++call_count, &result);
    return result;
  }
 private:
  size_t call_count = 0;
};

NAPI_MODULE_INIT() {
  return MyAddon::New(env, exports);
}

I'll check to see how best to integrate this with node-addon-api and its multiple ways of exposing bindings.

If you are looking for ā€œglobalā€ state that should be torn down when your addon is unloaded, then static data in your addon code should be the way to go.

I ran into some trouble when using static globals that are initialized in the before-main time on windows machines, at least while using cmakejs instead of gyp: https://github.com/cmake-js/cmake-js/issues/205 , however static local variables are working fine, so I recommend to use those.

However the whole context aware addon problematic came to my attention just of today, so my bug might be related to my probale misunderstanding of this.

Correct me if I am wrong: Since the whole shared library is loaded only once, at the electron app seems to be just one node-process (but multiple envs/worker threads /agents), I could use per process globals like static local variables for whole process granularity, as long as I do in a thread safe matter, right? Only draw back, would be the missing clean up. But at least I could have a global USB Device context, and queue the commands issued by my JS code.

@Superlokkus Yes, everything you said makes sense :+1:

@Superlokkus additionally, if you need per-addon-instance data, you can use napi_set_instance_data() and napi_get_instance_data() and you will get cleanup whenever an instance of your addon is unloaded. These are core N-APIs, but we're making them available in node-addon-api v3.0.0 (to be released soon) under Napi::Env::SetInstanceData() and Napi::Env::GetInstanceData().

https://github.com/nodejs/node-addon-api/blob/9c9accfbbe8c27f969d569f78758a8c47837321b/napi.h#L192-L204

Whoops ... no docs yet 😳

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

D-Andreev picture D-Andreev  Ā·  6Comments

fivdi picture fivdi  Ā·  8Comments

blagoev picture blagoev  Ā·  6Comments

NickNaso picture NickNaso  Ā·  3Comments

Wulf picture Wulf  Ā·  5Comments