SWIG_NAPI_IsWrappedObject check fails when multiple SWIG module is loaded

[Foolyou]

In every generated _wrap.cxx, Init will set its module-specific SWIG_NAPI_ObjectWrapCtor to a common env.GetInstanceData<EnvInstanceData>()->SWIG_NAPI_ObjectWrapCtor.
This will cause every former loaded SWIG module's instance method call fail.

[Foolyou]

Seems better to set SWIG_NAPI_ObjectWrapCtor on a per-module based object.

[Foolyou]

Also, the Napi::FunctionReference **ctor in EnvInstanceData seems also should be an per-module based object

[Foolyou]

I made some small changes to let swig jse work with this senario. Simply remove SWIG_NAPI_ObjectWrapCtor SWIG_NAPI_PackedObjectWrapCtor and ctor in EnvInstanceData and made a new ModuleClientData to hold those per-module things.
The ModuleClientDatas are attached on swig_modules' clientdata field.

It's working, but I'm not sure if this is a good practice.

[mmomtchev]

Yes, but ideally you want all the SWIG objects to inherit from the same base class.

Anyway, this has never been tested in JavaScript before, there are probably other things that are missing as well.

[Foolyou]

JSE currently use the EnvInstanceData to store swig_module_info list and other Env specific things. But it is easy to broken because we cannot prevent other library from calling napi_set_instance_data.
Also, if some version of JSE changed the structure of EnvInstanceData, modules created by different version of JSE may infect each other.

[mmomtchev]

napi_set_instance_data sets a separate pointer for each loaded module - otherwise no two Node.js modules would work when loaded at the same time.

What exactly is not working for you with the current version? The only thing that I see that is not working is inheritance of objects across modules. This does not require changing the swig_module_info structure which has been set in stone for decades.

Try the following test. First modify SWIG_NAPI_SetModule Lib/javascript/napi/javascriptinit.swg as follows:

SWIGRUNTIME void
SWIG_NAPI_SetModule(Napi::Env env, swig_module_info *swig_module) {
  auto data = new EnvInstanceData(env, swig_module);
  env.SetInstanceData(data);
  printf("Set instance %p data pointer to %p\n", (void *)((napi_env)env), swig_module);
}

Then pick up some random unit tests, run them separately so that the modules are all compiled, and then concatenate the _runme.js files into one. You will see:

Set instance 0x736de78cd290 data pointer to 0x736de785b4a0
Set instance 0x6a49760 data pointer to 0x736de74ff560
Set instance 0x736de78e59c0 data pointer to 0x736de74ce420

The NAPI contexts of the modules are completely separate and cannot be accessed across the module boundary.

If one day we want to be able to support cross-module inheritance and general types reuse, I think that still can be done without changing the structure. Modules that support this will concatenate their types.

[Foolyou]

I have tried your test, and yes the env and instance data is different across modules.

But It's wired that if instance data is different across modules, my first problem of

Init will set its module-specific SWIG_NAPI_ObjectWrapCtor to a common env.GetInstanceData()->SWIG_NAPI_ObjectWrapCtor.

should not happen.

And actually I tried that in two new simple swig modules in nodejs, the thing I said does not happened.

Now I'm thinking may be the problem is because my code is not running on real node API. It's running on a new Phone OS which use a node N-API "compatible" API to develop native modules to be called by typescript(which handles most view logic).

Currently I does not have the device, tomorrow I will try it again and maybe this is not the case in a common Node N-API evironment.

[mmomtchev]

What runtime do you use? Since obviously it is not Node.js?

If this implementation returns a shared pointer for all modules, that would be a major Node-API incompatibility. It would be somewhat surprising. Maybe there is more to it.

[Foolyou]

It's currently a experimental runtime and not publicly avalible which called HarmonyOS Native API.
I have found lots of incompatible things on its so-called "N-API compatible" APIs 👀 , and tried a lot to fill the gap.

Just a moment ago I tried more on real node N-API, and found this is not a problem on that.
I think I have to do lots of work to deal with the incompatibility of HarmonyOS's API.

Thank you very much for your help. After the runtime is avalible on market I will try to contribute my port as open-sourced.

[Foolyou]

Upon evaluating the HarmonyOS Node-API, it was confirmed that the napi_env remains unchanged across different modules within the same JavaScript thread. I have now utilized a thread_local variable to store EnvInstanceData. With some minor modifications, it now operates successfully on HarmonyOS.

Additionally, HarmonyOS may load a native module multiple times within a single thread when encountering an "XComponent" (basically a JavaScript UI Component that uses a native module as its backend). Hence, it is necessary to check for the existence of previously created class constructors. If they are present, I must reuse them to ensure consistency.

The code is still undergoing testing in more modules. Once it stabilizes, I will release a fork. Considering HarmonyOS's current status as not open, stable, or widely adopted as a runtime environment, I think it would be better not to send a pull request your way at this moment.