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

Cannot create multiple NodeJSEnvironment for one NodeJSPlatform #348

Open
camnewnham opened this issue Aug 6, 2024 · 7 comments
Open

Cannot create multiple NodeJSEnvironment for one NodeJSPlatform #348

camnewnham opened this issue Aug 6, 2024 · 7 comments
Assignees
Labels
Milestone

Comments

@camnewnham
Copy link

The docs suggest that multiple environments can exist concurrently for the sample platform.

I get an assertion failure from this simple console application:

using Microsoft.JavaScript.NodeApi.Runtime;
NodejsPlatform platform = new NodejsPlatform("libnode.dll");
NodejsEnvironment env1 = platform.CreateEnvironment();
NodejsEnvironment env2 = platform.CreateEnvironment();
C:\Users\me\Desktop\libnode\src\inspector_agent.cc:704: Assertion `(start_io_thread_async_initialized.exchange(true)) == (false)' failed.
 1: 00007FF961841CAF v8_inspector::V8StackTrace::V8StackTrace+649598
...

libnode was built with the instructions and is using the suggested napi-libnode-v20.9.0 branch.

@jasongin
Copy link
Member

jasongin commented Aug 6, 2024

I can reproduce this. While destroying one environment and then creating another will work, creating a second concurrent environment crashes.

@vmoroz It should be supported, right? Can you try adding a test case for this in the libnode PR?

@jasongin jasongin added this to the Embedding milestone Aug 14, 2024
@vmoroz
Copy link
Member

vmoroz commented Sep 10, 2024

@camnewnham , I am currently working on the new version of the libnode API and I have faced the same issue. The root cause is that the Inspector (JS debugger) can be associated only with a single root environment. Using multiple environment requires to explicitly disable the Inspector. There is a test/embedding/embedtest_concurrent_node_api.cc file to test the multiple runtimes in the PR.

The issue should be addressed in the new version of the API. The plan is adopt it in the node-api-dotnet project as soon as we complete the key outstanding TODOs. I.e. maybe even before the PR is merged. This is to be sure sure that the new API can be integrated well with .Net.

@vmoroz
Copy link
Member

vmoroz commented Sep 10, 2024

@camnewnham , I am also looking for real life scenarios for the libnode API to make sure that it addresses the real needs.
I wonder if you could share high level ideas about what you would like to get from that API.

@camnewnham
Copy link
Author

camnewnham commented Sep 16, 2024

The specific scenario I have for multiple environments is:

  • A dotnet host program utilises a visual node-graph editor
  • Each node in the editor is associated with a .js file
  • When .js file is edited, the owner node reloads and re-executes any dependencies.

Currently each node is in a shared environment. The current behaviour is:

  1. A JS file is edited
  2. The environment is disposed and recreated
  3. The JS file is imported
  4. The node executes itself and any dependencies

The problem being that (2) interferes with other nodes which may have independent timeouts (related: #355) or cached data. Ideally, the environment would be unique to the node and editing one would be isolated from any other.

@vmoroz
Copy link
Member

vmoroz commented Sep 16, 2024

Thank you, @camnewnham , for sharing your scenario!
Just a few clarifying questions:

  • My understanding is that you may have or want to have several Node.js environments in your app simultaneously, is it right?
  • Should they each run in their own thread, or you would prefer them to be using the same thread?
  • If it is the same thread, then do you want them to run in the app's UI thread?

@camnewnham
Copy link
Author

  • My understanding is that you may have or want to have several Node.js environments in your app simultaneously, is it right?

Yes, that's correct. They will reasonably frequently be created and disposed.

  • Should they each run in their own thread, or you would prefer them to be using the same thread?
  • If it is the same thread, then do you want them to run in the app's UI thread?

I don't have a strong preference here, but expectations based on the current NodeJsEnvironment.Run pattern would be that each environment has a separate thread. Are there any other implications?

@vmoroz
Copy link
Member

vmoroz commented Sep 17, 2024

Running each Node.js environment in its own thread is generally fine.
The code may become complex when the JS must interact with the .Net code that has its own threading requirements.
In the new API I would like to support four different modes:

  • Each environment is in its own thread.
  • Multiple environments are in the same thread.
  • The same environment can be run from multiple threads as long as they are run sequentially. (e.g. we can use the .Net Tasks for it.)
  • The environment is run from UI message loop, UI dispatcher, or some kind of event loop associated with a thread.

We must be able to mix and match different modes.
There are tests running for these cases.
Though the last mode is Windows-only so far - the other platforms are coming soon.

I was just wondering if your case fits into these categories, and so far it seems that it must be covered.

Another aspect is that there can be only a single debuggable Node.js root environment per process.
The Inspector can be inherited by the dependent worker threads.
I still must investigate the details about the Inspector support in the new API.

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

No branches or pull requests

3 participants