Skip to content

Use UnmanagedCallersOnly in AssemblyDependencyResolver. #119034

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use UnmanagedCallersOnly in AssemblyDependencyResolver. #119034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d73f4a8
Use `UnmanagedCallersOnly` in `AssemblyDependencyResolver`.
teo-tsirpanis Aug 23, 2025
2166c72
Fix test that relied on internal types.
teo-tsirpanis Aug 23, 2025
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
15 changes: 5 additions & 10 deletions src/libraries/Common/src/Interop/Interop.HostPolicy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@
using System;
using System.Runtime.InteropServices;

using unsafe ErrorWriterCallback = delegate* unmanaged[Cdecl]<nint, void>;

internal static partial class Interop
{
internal static partial class HostPolicy
internal static unsafe partial class HostPolicy
{
[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
internal delegate void corehost_resolve_component_dependencies_result_fn(IntPtr assemblyPaths,
IntPtr nativeSearchPaths, IntPtr resourceSearchPaths);

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
internal delegate void corehost_error_writer_fn(IntPtr message);

#pragma warning disable CS3016 // Arrays as attribute arguments is not CLS-compliant
#if TARGET_WINDOWS
[LibraryImport(Libraries.HostPolicy, StringMarshalling = StringMarshalling.Utf16)]
Expand All @@ -23,11 +18,11 @@ internal delegate void corehost_resolve_component_dependencies_result_fn(IntPtr
#endif
[UnmanagedCallConv(CallConvs = [typeof(System.Runtime.CompilerServices.CallConvCdecl)])]
internal static partial int corehost_resolve_component_dependencies(string componentMainAssemblyPath,
corehost_resolve_component_dependencies_result_fn result);
delegate* unmanaged[Cdecl]<nint, nint, nint, void> result);

[LibraryImport(Libraries.HostPolicy)]
[UnmanagedCallConv(CallConvs = [typeof(System.Runtime.CompilerServices.CallConvCdecl)])]
internal static partial IntPtr corehost_set_error_writer(IntPtr errorWriter);
internal static partial ErrorWriterCallback corehost_set_error_writer(ErrorWriterCallback errorWriter);
#pragma warning restore CS3016 // Arrays as attribute arguments is not CLS-compliant
}
}
89 changes: 57 additions & 32 deletions src/libraries/System.Private.CoreLib/src/System/Runtime/Loader/AssemblyDependencyResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Versioning;
using System.Text;
Expand Down Expand Up @@ -35,39 +37,32 @@ public AssemblyDependencyResolver(string componentAssemblyPath)
{
ArgumentNullException.ThrowIfNull(componentAssemblyPath);

string? assemblyPathsList = null;
string? nativeSearchPathsList = null;
string? resourceSearchPathsList = null;
int returnCode = 0;

StringBuilder errorMessage = new StringBuilder();
ThreadLocalState state = new ThreadLocalState();
Debug.Assert(t_threadLocalState == null); // Re-entrant calls should not happen
Comment on lines +42 to +43
Copy link
Preview

Copilot AI Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Debug.Assert for re-entrant calls may not be sufficient protection in release builds. Consider throwing an exception or implementing proper re-entrancy handling, as the assertion is compiled out in release mode and concurrent access could corrupt the thread-local state.

Suggested change
ThreadLocalState state = new ThreadLocalState();
Debug.Assert(t_threadLocalState == null); // Re-entrant calls should not happen
if (t_threadLocalState != null)
throw new InvalidOperationException("Re-entrant calls to AssemblyDependencyResolver are not allowed.");

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary; the callback will not run arbitrary code, but a specific function provided by the framework.

t_threadLocalState = state;
try
{
// Setup error writer for this thread. This makes the hostpolicy redirect all error output
// to the writer specified. Have to store the previous writer to set it back once this is done.
var errorWriter = new Interop.HostPolicy.corehost_error_writer_fn(message => errorMessage.AppendLine(Marshal.PtrToStringAuto(message)));

IntPtr errorWriterPtr = Marshal.GetFunctionPointerForDelegate(errorWriter);
IntPtr previousErrorWriterPtr = Interop.HostPolicy.corehost_set_error_writer(errorWriterPtr);

try
unsafe
{
// Call hostpolicy to do the actual work of finding .deps.json, parsing it and extracting
// information from it.
returnCode = Interop.HostPolicy.corehost_resolve_component_dependencies(
componentAssemblyPath,
(assemblyPaths, nativeSearchPaths, resourceSearchPaths) =>
{
assemblyPathsList = Marshal.PtrToStringAuto(assemblyPaths);
nativeSearchPathsList = Marshal.PtrToStringAuto(nativeSearchPaths);
resourceSearchPathsList = Marshal.PtrToStringAuto(resourceSearchPaths);
});
}
finally
{
// Reset the error write to the one used before
Interop.HostPolicy.corehost_set_error_writer(previousErrorWriterPtr);
GC.KeepAlive(errorWriter);
// Setup error writer for this thread. This makes the hostpolicy redirect all error output
// to the writer specified. Have to store the previous writer to set it back once this is done.
var previousErrorWriterPtr = Interop.HostPolicy.corehost_set_error_writer(&ErrorWriterCallback);

try
{
// Call hostpolicy to do the actual work of finding .deps.json, parsing it and extracting
// information from it.
returnCode = Interop.HostPolicy.corehost_resolve_component_dependencies(componentAssemblyPath, &ResolveComponentDependenciesCallback);
}
finally
{
// Reset the error write to the one used before
Interop.HostPolicy.corehost_set_error_writer(previousErrorWriterPtr);
// Clear thread-local state
t_threadLocalState = null;
}
}
}
catch (EntryPointNotFoundException entryPointNotFoundException)
Expand All @@ -86,10 +81,10 @@ public AssemblyDependencyResolver(string componentAssemblyPath)
SR.AssemblyDependencyResolver_FailedToResolveDependencies,
componentAssemblyPath,
returnCode,
errorMessage));
state.ErrorMessage));
}

string[] assemblyPaths = SplitPathsList(assemblyPathsList);
string[] assemblyPaths = SplitPathsList(state.AssemblyPathsList);

// Assembly simple names are case insensitive per the runtime behavior
// (see SimpleNameToFileNameMapTraits for the TPA lookup hash).
Expand All @@ -101,8 +96,8 @@ public AssemblyDependencyResolver(string componentAssemblyPath)
_assemblyPaths.TryAdd(Path.GetFileNameWithoutExtension(assemblyPath), assemblyPath);
}

_nativeSearchPaths = SplitPathsList(nativeSearchPathsList);
_resourceSearchPaths = SplitPathsList(resourceSearchPathsList);
_nativeSearchPaths = SplitPathsList(state.NativeSearchPathsList);
_resourceSearchPaths = SplitPathsList(state.ResourceSearchPathsList);

_assemblyDirectorySearchPaths = [Path.GetDirectoryName(componentAssemblyPath)!];
}
Expand Down Expand Up @@ -200,5 +195,35 @@ private static string[] SplitPathsList(string? pathsList)
return pathsList.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries);
}
}

#pragma warning disable CS3016 // Arrays as attribute arguments is not CLS-compliant
[UnmanagedCallersOnly(CallConvs = [typeof(CallConvCdecl)])]
private static void ResolveComponentDependenciesCallback(nint assemblyPaths, nint nativeSearchPaths, nint resourceSearchPaths)
{
ThreadLocalState? state = t_threadLocalState;
Debug.Assert(state != null);
Comment on lines +203 to +204
Copy link
Preview

Copilot AI Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Debug.Assert may not provide adequate protection in release builds. If t_threadLocalState is null, the callback will fail silently or cause a NullReferenceException on the next line. Consider using ArgumentNullException.ThrowIf or similar exception throwing mechanism.

Suggested change
ThreadLocalState? state = t_threadLocalState;
Debug.Assert(state != null);
ArgumentNullException.ThrowIfNull(state);

Copilot uses AI. Check for mistakes.

state.AssemblyPathsList = Marshal.PtrToStringAuto(assemblyPaths);
state.NativeSearchPathsList = Marshal.PtrToStringAuto(nativeSearchPaths);
state.ResourceSearchPathsList = Marshal.PtrToStringAuto(resourceSearchPaths);
}

[UnmanagedCallersOnly(CallConvs = [typeof(CallConvCdecl)])]
private static void ErrorWriterCallback(nint message)
{
ThreadLocalState? state = t_threadLocalState;
Debug.Assert(state != null);
Comment on lines +213 to +214
Copy link
Preview

Copilot AI Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Debug.Assert may not provide adequate protection in release builds. If t_threadLocalState is null, the callback will fail silently or cause a NullReferenceException on the next line. Consider using ArgumentNullException.ThrowIf or similar exception throwing mechanism.

Suggested change
ThreadLocalState? state = t_threadLocalState;
Debug.Assert(state != null);
if (state is null)
throw new InvalidOperationException("Thread local state is not set in ErrorWriterCallback.");

Copilot uses AI. Check for mistakes.

state.ErrorMessage ??= new StringBuilder();
state.ErrorMessage.AppendLine(Marshal.PtrToStringAuto(message));
}
#pragma warning restore CS3016 // Arrays as attribute arguments is not CLS-compliant

[ThreadStatic]
private static ThreadLocalState? t_threadLocalState;

private sealed class ThreadLocalState
{
public StringBuilder? ErrorMessage;
public string? AssemblyPathsList, NativeSearchPathsList, ResourceSearchPathsList;
}
}
}
24 changes: 10 additions & 14 deletions src/tests/Common/CoreCLRTestLibrary/HostPolicyMock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,11 @@ internal delegate void Callback_corehost_resolve_component_dependencies(
private static extern void Set_corehost_resolve_component_dependencies_Callback(
IntPtr callback);

private static Type _corehost_error_writer_fnType;

[UnmanagedFunctionPointer(CallingConvention.Cdecl, CharSet = CharSet.Auto)]
public delegate void ErrorWriterDelegate(string message);

public static void Initialize(string testBasePath, string coreRoot)
{
// This is needed for marshalling of function pointers to work - requires private access to the ADR unfortunately
// Delegate marshalling doesn't support casting delegates to anything but the original type
// so we need to use the original type.
_corehost_error_writer_fnType = typeof(object).Assembly.GetType("Interop+HostPolicy+corehost_error_writer_fn");
}

public static MockValues_corehost_resolve_component_dependencies Mock_corehost_resolve_component_dependencies(
Expand Down Expand Up @@ -125,17 +119,19 @@ public Action<string> LastSetErrorWriter
}
else
{
Delegate d = Marshal.GetDelegateForFunctionPointer(errorWriterPtr, _corehost_error_writer_fnType);
return (string message) =>
{
IntPtr messagePtr = Marshal.StringToCoTaskMemAuto(message);
try
{
d.DynamicInvoke(messagePtr);
}
finally
unsafe
{
Marshal.FreeCoTaskMem(messagePtr);
IntPtr messagePtr = Marshal.StringToCoTaskMemAuto(message);
try
{
((delegate* unmanaged[Cdecl]<nint, void>)errorWriterPtr)(messagePtr);
}
finally
{
Marshal.FreeCoTaskMem(messagePtr);
}
}
};
}
Expand Down
Loading