Skip to content

Fix worker assembly loading bug #448

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

Merged
merged 3 commits into from
Jun 26, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ dotnet_diagnostic.CA1835.severity = none

dotnet_diagnostic.IDE0290.severity = none # Use primary constructor
dotnet_diagnostic.IDE0065.severity = none # Using directives must be placed outside of namespace
dotnet_diagnostic.IDE0350.severity = none # Lambda expression can be simplified

# Collection initialization can be simplified.
# This relies on implicit conversions in ways that are sometimes undesirable.
Expand Down
62 changes: 33 additions & 29 deletions src/NodeApi.DotNetHost/ManagedHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
Expand Down Expand Up @@ -49,13 +50,13 @@ public sealed class ManagedHost : JSEventEmitter, IDisposable
/// <summary>
/// Mapping from assembly file paths to loaded assemblies.
/// </summary>
private readonly Dictionary<string, Assembly> _loadedAssembliesByPath = new();
private readonly ConcurrentDictionary<string, Assembly?> _loadedAssembliesByPath = new();

/// <summary>
/// Mapping from assembly names (not including version or other parts) to
/// loaded assemblies.
/// </summary>
private readonly Dictionary<string, Assembly> _loadedAssembliesByName = new();
private readonly ConcurrentDictionary<string, Assembly> _loadedAssembliesByName = new();

/// <summary>
/// Tracks names of assemblies that have been exported to JS.
Expand Down Expand Up @@ -467,7 +468,7 @@ public JSValue LoadAssembly(JSCallbackArgs args)
assembly = LoadAssembly(assemblyNameOrFilePath, allowNativeLibrary: true);
}

if (!_exportedAssembliesByName.Contains(assembly.GetName().Name!))
if (assembly != null && !_exportedAssembliesByName.Contains(assembly.GetName().Name!))
{
_typeExporter.ExportAssemblyTypes(assembly);
_exportedAssembliesByName.Add(assembly.GetName().Name!);
Expand All @@ -487,7 +488,7 @@ private JSValue ResolveAssembly(JSCallbackArgs args)
return default;
}

private Assembly LoadAssembly(string assemblyNameOrFilePath, bool allowNativeLibrary)
private Assembly? LoadAssembly(string assemblyNameOrFilePath, bool allowNativeLibrary)
{
Trace($"> ManagedHost.LoadAssembly({assemblyNameOrFilePath})");

Expand Down Expand Up @@ -525,41 +526,44 @@ private Assembly LoadAssembly(string assemblyNameOrFilePath, bool allowNativeLib
"or the name of a system assembly (without path or DLL extension).");
}

Assembly assembly;
try
Assembly? assembly = _loadedAssembliesByPath.GetOrAdd(assemblyFilePath, _ =>
{
try
{
#if NETFRAMEWORK || NETSTANDARD
// TODO: Load assemblies in a separate appdomain.
assembly = Assembly.LoadFrom(assemblyFilePath);
// TODO: Load assemblies in a separate appdomain.
return Assembly.LoadFrom(assemblyFilePath);
#else
assembly = _loadContext.LoadFromAssemblyPath(assemblyFilePath);
return _loadContext.LoadFromAssemblyPath(assemblyFilePath);
#endif
}
catch (BadImageFormatException)
{
if (!allowNativeLibrary)
{
throw;
}
catch (BadImageFormatException)
{
if (!allowNativeLibrary)
{
throw;
}

// This might be a native DLL, not a managed assembly.
// Load the native library, which enables it to be auto-resolved by
// any later DllImport operations for the same library name.
NativeLibrary.Load(assemblyFilePath);
// This might be a native DLL, not a managed assembly.
// Load the native library, which enables it to be auto-resolved by
// any later DllImport operations for the same library name.
NativeLibrary.Load(assemblyFilePath);
return null;
}
catch (FileNotFoundException fnfex)
{
throw new FileNotFoundException(
$"Assembly file not found: {assemblyNameOrFilePath}", fnfex);
}
});

Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath} (native library)");
return null!;
}
catch (FileNotFoundException fnfex)
if (assembly != null)
{
throw new FileNotFoundException(
$"Assembly file not found: {assemblyNameOrFilePath}", fnfex);
_loadedAssembliesByName.GetOrAdd(assembly.GetName().Name!, assembly);
}

_loadedAssembliesByPath.Add(assemblyFilePath, assembly);
_loadedAssembliesByName.Add(assembly.GetName().Name!, assembly);

Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath}, {assembly.GetName().Version}");
string version = assembly?.GetName().Version?.ToString() ?? "(native library)";
Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath} {version}");
return assembly;
}

Expand Down