Skip to content

Commit 437b69f

Browse files
authored
Fix worker assembly loading bug (#448)
1 parent de85539 commit 437b69f

File tree

2 files changed

+34
-29
lines changed

2 files changed

+34
-29
lines changed

.editorconfig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ dotnet_diagnostic.CA1835.severity = none
161161

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

165166
# Collection initialization can be simplified.
166167
# This relies on implicit conversions in ways that are sometimes undesirable.

src/NodeApi.DotNetHost/ManagedHost.cs

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using System;
5+
using System.Collections.Concurrent;
56
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.Globalization;
@@ -49,13 +50,13 @@ public sealed class ManagedHost : JSEventEmitter, IDisposable
4950
/// <summary>
5051
/// Mapping from assembly file paths to loaded assemblies.
5152
/// </summary>
52-
private readonly Dictionary<string, Assembly> _loadedAssembliesByPath = new();
53+
private readonly ConcurrentDictionary<string, Assembly?> _loadedAssembliesByPath = new();
5354

5455
/// <summary>
5556
/// Mapping from assembly names (not including version or other parts) to
5657
/// loaded assemblies.
5758
/// </summary>
58-
private readonly Dictionary<string, Assembly> _loadedAssembliesByName = new();
59+
private readonly ConcurrentDictionary<string, Assembly> _loadedAssembliesByName = new();
5960

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

470-
if (!_exportedAssembliesByName.Contains(assembly.GetName().Name!))
471+
if (assembly != null && !_exportedAssembliesByName.Contains(assembly.GetName().Name!))
471472
{
472473
_typeExporter.ExportAssemblyTypes(assembly);
473474
_exportedAssembliesByName.Add(assembly.GetName().Name!);
@@ -487,7 +488,7 @@ private JSValue ResolveAssembly(JSCallbackArgs args)
487488
return default;
488489
}
489490

490-
private Assembly LoadAssembly(string assemblyNameOrFilePath, bool allowNativeLibrary)
491+
private Assembly? LoadAssembly(string assemblyNameOrFilePath, bool allowNativeLibrary)
491492
{
492493
Trace($"> ManagedHost.LoadAssembly({assemblyNameOrFilePath})");
493494

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

528-
Assembly assembly;
529-
try
529+
Assembly? assembly = _loadedAssembliesByPath.GetOrAdd(assemblyFilePath, _ =>
530530
{
531+
try
532+
{
531533
#if NETFRAMEWORK || NETSTANDARD
532-
// TODO: Load assemblies in a separate appdomain.
533-
assembly = Assembly.LoadFrom(assemblyFilePath);
534+
// TODO: Load assemblies in a separate appdomain.
535+
return Assembly.LoadFrom(assemblyFilePath);
534536
#else
535-
assembly = _loadContext.LoadFromAssemblyPath(assemblyFilePath);
537+
return _loadContext.LoadFromAssemblyPath(assemblyFilePath);
536538
#endif
537-
}
538-
catch (BadImageFormatException)
539-
{
540-
if (!allowNativeLibrary)
541-
{
542-
throw;
543539
}
540+
catch (BadImageFormatException)
541+
{
542+
if (!allowNativeLibrary)
543+
{
544+
throw;
545+
}
544546

545-
// This might be a native DLL, not a managed assembly.
546-
// Load the native library, which enables it to be auto-resolved by
547-
// any later DllImport operations for the same library name.
548-
NativeLibrary.Load(assemblyFilePath);
547+
// This might be a native DLL, not a managed assembly.
548+
// Load the native library, which enables it to be auto-resolved by
549+
// any later DllImport operations for the same library name.
550+
NativeLibrary.Load(assemblyFilePath);
551+
return null;
552+
}
553+
catch (FileNotFoundException fnfex)
554+
{
555+
throw new FileNotFoundException(
556+
$"Assembly file not found: {assemblyNameOrFilePath}", fnfex);
557+
}
558+
});
549559

550-
Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath} (native library)");
551-
return null!;
552-
}
553-
catch (FileNotFoundException fnfex)
560+
if (assembly != null)
554561
{
555-
throw new FileNotFoundException(
556-
$"Assembly file not found: {assemblyNameOrFilePath}", fnfex);
562+
_loadedAssembliesByName.GetOrAdd(assembly.GetName().Name!, assembly);
557563
}
558564

559-
_loadedAssembliesByPath.Add(assemblyFilePath, assembly);
560-
_loadedAssembliesByName.Add(assembly.GetName().Name!, assembly);
561-
562-
Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath}, {assembly.GetName().Version}");
565+
string version = assembly?.GetName().Version?.ToString() ?? "(native library)";
566+
Trace($"< ManagedHost.LoadAssembly() => {assemblyFilePath} {version}");
563567
return assembly;
564568
}
565569

0 commit comments

Comments
 (0)