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

Host throws exception when initialized multiple times by JS workers #93

Closed
jasongin opened this issue Apr 12, 2023 · 7 comments · Fixed by #95 or #96
Closed

Host throws exception when initialized multiple times by JS workers #93

jasongin opened this issue Apr 12, 2023 · 7 comments · Fixed by #95 or #96
Assignees
Labels
bug Something isn't working

Comments

@jasongin
Copy link
Member

jasongin commented Apr 12, 2023

The node-api-dotnet native host throws an exception if its initialize() function is called multiple times in the same process:

.NET is already initialized in the current process. Initializing multiple .NET versions is not supported.

Normally a JS require() will return the same object if called more than once for the same module path. But multiple initializations can happen for either of two reasons:

  1. Multiple .NET TFMs are requested in the same process, e.g. require('node-api-dotnet/net6.0') and require('node-api-dotnet/net7.0').
  2. The module is required (with the same path) from multiple JS workers. Each worker initializes the module separately.

The first case is a valid error, since, as the error message says, multiple .NET versions in the same process are not supported.

But the second case should work. The native host can check if the same .NET version is requested and if so the re-initialization can be allowed. Then the managed host should use a separate assembly load context for each worker.

@jasongin jasongin self-assigned this Apr 12, 2023
@jasongin jasongin added the bug Something isn't working label Apr 12, 2023
@jasongin jasongin changed the title Host throws exception when initialized multiple times by workers Host throws exception when initialized multiple times by JS workers Apr 12, 2023
@rijulg
Copy link
Contributor

rijulg commented Apr 13, 2023

Just encountered this while calling load just once in a github action.
I suspect the github environment has an already runing .Net session which doesn't even let us initalize this module once.

My dependency graph was actually calling the load function more than once while running tests on github actions, so it turns out I encountered the exact same bug. Might be worth noting that it only happened on github and not local.

@jasongin
Copy link
Member Author

My dependency graph was actually calling the load function more than once

That should be fine. Subsequent calls to load the same assembly file will just immediately return the already-loaded assembly object.

After looking into this further, I'm not sure my analysis above of the root cause was correct. We actually have a test case that loads .NET from multiple workers in a process, and it works. I'm unable to reproduce the error (except using private APIs that you wouldn't be calling).

Can you share:

  • The version of node-api-dotnet package you're using
  • Portion(s) of your JS code that use that package - note I do not have access to Office org private repos.

@rijulg
Copy link
Contributor

rijulg commented Apr 14, 2023

So I have replicated my issue in a public repo (https://github.com/rijulg/node-api-dotnet-issue), unfortunately even the simple example is a bit involved as the bug I encountered was in a project using nestjs so bear with me. I have enapsulated all my project requirements in a devcontainer for running locally, so it should be easy to run locally if you need to.

Essentially we see the following in github actions (but not when running locally):

  1. Jest runs the src/repositories/DLL.spec.ts test which loads the dll
  2. Jest runs the src/controllers/Example.spec.ts test which has a dependency on the repositories
    2.1. Nestjs loads the dependency internally and for that it creates an instance of DLLRepsository
    2.2. A new instance of DLLRepository tries to load the dll and fails with the following error:

.NET is already initialized in the current process. Initializing multiple .NET versions is not supported.

You can see the issue over at this github action report:
https://github.com/rijulg/node-api-dotnet-issue/actions/runs/4696432987/jobs/8326519102

@jasongin
Copy link
Member Author

This is very likely nodejs/node#6624, or at least related to it. I happen to have investigated an issue for another project a couple years ago that I traced to Node.js mixing up the drive letter case in the context of a test runner, which led to some modules being loaded multiple times when they shouldn't be. (So I know it's not fully fixed in Node.js.) And there are comments from others specifically mentioning it is a problem with jest.

#95 should fix it.

@rijulg
Copy link
Contributor

rijulg commented Apr 14, 2023

At least the error changed: https://github.com/rijulg/node-api-dotnet-issue/actions/runs/4697994667/jobs/8329728914
I am getting the following error now:

Object reference not set to an instance of an object.

while running the following in second test

import { load as loadDLL } from "node-api-dotnet";

Still the error is just on github actions and not locally which is weird.

@jasongin
Copy link
Member Author

The NRE is fixed by #96

I was able to repro and test it better this time.

@rijulg
Copy link
Contributor

rijulg commented Apr 14, 2023

Yup, the new fix works
https://github.com/rijulg/node-api-dotnet-issue/actions/runs/4699042432/jobs/8332048634

Waiting on merge so that I can update my dependncies to the main branch. Thank you so much Jason!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants