Improve FileNotFoundException diagnostics when loading a TPA assembly fails#126782
Improve FileNotFoundException diagnostics when loading a TPA assembly fails#126782elinor-fung wants to merge 11 commits intodotnet:mainfrom
Conversation
Thread diagnostic context through the assembly binding pipeline so that FileNotFoundException carries actionable information about what failed and where, instead of just a generic message. Key changes: - Populate FusionLog on FileNotFoundException with binding diagnostic info (file open errors, metadata read failures, cached failure replays) - Preserve original Win32 error through BinderAcquirePEImage instead of discarding it via EX_CATCH_HRESULT - Fix TryOpenFile to return ERROR_OPEN_FAILED (not ERROR_FILE_NOT_FOUND) when GetLastError() is zero, making the error distinguishable - Extend EEFileLoadException with m_diagnosticInfo field and thread it through CreateThrowable to the managed FileLoadException.Create bridge - Extend FailureCacheEntry with diagnostic context so cached failure replays include the original error info with a '(Cached)' prefix - Thread SString* pDiagnosticInfo through the bind chain: BindAssembly -> BindByName -> BindLocked -> BindByTpaList -> GetAssembly - All diagnostic message format strings are in mscorrc.rc resources All diagnostic string formatting occurs only in error paths. The happy path has zero additional allocations or string work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Thread SString* pDiagnosticInfo through: AssemblyBinder::BindAssemblyByName -> BindUsingAssemblyName (virtual, Default + Custom overrides) -> BindAssemblyByNameWorker -> AssemblyBinderCommon::BindAssembly -> AssemblySpec::Bind -> AppDomain::BindAssemblySpec Add EEFileLoadException::Throw overload accepting diagnostic SString. In AppDomain::BindAssemblySpec, pass StackSString to Bind() and use it at the EEFileLoadException::Throw site so the diagnostic info reaches FileNotFoundException.FusionLog on the managed side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the original ERROR_FILE_NOT_FOUND fallback when GetLastError() is zero. The GetLastError() caching fix (using a local variable) is kept. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace FormatMessage (%1, %2) with Printf-style (%s, %08x) format strings in mscorrc.rc resources. Use AppendPrintf instead of FormatMessage/Set so diagnostic info accumulates across multiple failure points rather than overwriting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use GetHRMsg to convert HRESULTs to human-readable messages (e.g. 'The process cannot access the file because it is being used by another process') instead of raw hex codes. Update cached failure prefix to '[cached failure]'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add TpaBindFailureTest with three scenarios, each using a dedicated TPA assembly referenced as a normal ProjectReference: - TpaAssembly_NotFound: delete the DLL, use the type via [NoInlining] wrapper, verify exception string contains the assembly path - TpaAssembly_SharingViolation: lock the DLL exclusively, use the type, verify exception string contains the file path and HRESULT 80070020 - TpaAssembly_Corrupt: overwrite with garbage bytes, use the type, verify exception string contains the file path Each scenario triggers the load via direct type usage (not explicit Assembly.Load) to exercise the realistic JIT/type-system load path through BindByTpaList. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tagging subscribers to this area: @agocke, @elinor-fung |
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostics when assembly loading fails by threading native binder failure context through the VM into managed FileNotFoundException.FusionLog, making it clearer which step failed, which path was involved, and which HRESULT caused the failure.
Changes:
- Thread a diagnostic string through the native binder pipeline (including failure caching) and into
EEFileLoadException, then across the unmanaged→managed bridge. - Populate
FileNotFoundException.FusionLogfrom native diagnostic context (including cached-failure labeling). - Add
System.Runtime.Loadertests and support assemblies to validate path/HRESULT presence for missing/locked/corrupt TPA scenarios.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Runtime.Loader/tests/TpaLoadFailureTest.cs | Adds new tests asserting exception output includes failing path + HRESULT for key TPA failure modes. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj | Wires in new test + support projects; deletes “Missing/Corrupt” DLLs post-build so they stay in deps/TPA but not on disk. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Missing/TestClass.cs | Support assembly used to trigger “missing on disk” bind failure. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Missing/System.Runtime.Loader.Test.BindFailure.Missing.csproj | Defines the “Missing” support project. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Locked/TestClass.cs | Support assembly used to trigger sharing-violation bind failure. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Locked/System.Runtime.Loader.Test.BindFailure.Locked.csproj | Defines the “Locked” support project. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Corrupt/TestClass.cs | Support assembly used to trigger corrupt-image bind failure. |
| src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Test.BindFailure.Corrupt/System.Runtime.Loader.Test.BindFailure.Corrupt.csproj | Defines the “Corrupt” support project. |
| src/libraries/System.Private.CoreLib/src/System/IO/FileNotFoundException.cs | Makes FusionLog internally settable so CoreLib can attach diagnostics. |
| src/coreclr/System.Private.CoreLib/src/System/IO/FileNotFoundException.CoreCLR.cs | Adds internal ctor that accepts diagnostic info and assigns it to FusionLog. |
| src/coreclr/System.Private.CoreLib/src/System/IO/FileLoadException.CoreCLR.cs | Extends unmanaged bridge to accept a diagnostic string pointer and pass it into FileNotFoundException. |
| src/coreclr/vm/clrex.h | Extends EEFileLoadException to carry a diagnostic SString and adds a Throw overload. |
| src/coreclr/vm/clrex.cpp | Passes diagnostic info across the FileLoadException.Create unmanaged-caller boundary; adds ctor/Throw overload implementation. |
| src/coreclr/vm/appdomain.cpp | Threads diagnostic info through AssemblySpec::Bind and uses it when throwing EEFileLoadException for file-not-found binds. |
| src/coreclr/vm/assemblyspec.hpp | Updates AssemblySpec::Bind signature to optionally accept diagnostic info. |
| src/coreclr/vm/coreassemblyspec.cpp | Captures open-file failures/exceptions with path + HRESULT/message into the diagnostic string. |
| src/coreclr/vm/assemblybinder.h | Extends binder APIs to optionally accept diagnostic info. |
| src/coreclr/vm/assemblybinder.cpp | Forwards diagnostic info through BindAssemblyByName to BindUsingAssemblyName. |
| src/coreclr/vm/peimage.cpp | Avoids double GetLastError() call when converting to HRESULT. |
| src/coreclr/dlls/mscorrc/resource.h | Adds resource IDs for new binder diagnostic format strings. |
| src/coreclr/dlls/mscorrc/mscorrc.rc | Adds diagnostic format strings used by the binder for FusionLog content. |
| src/coreclr/binder/inc/assemblybindercommon.hpp | Threads SString* pDiagnosticInfo through binder common APIs. |
| src/coreclr/binder/assemblybindercommon.cpp | Records metadata init failures into diagnostics and propagates diagnostic info through bind steps. |
| src/coreclr/binder/inc/failurecachehashtraits.hpp | Extends FailureCacheEntry to store diagnostic info with cached failures. |
| src/coreclr/binder/inc/failurecache.hpp | Extends failure cache APIs to store/return diagnostic strings. |
| src/coreclr/binder/failurecache.cpp | Stores diagnostic info in the failure cache and prefixes diagnostics when replaying cached failures. |
| src/coreclr/binder/inc/applicationcontext.hpp | Extends failure-cache integration to accept diagnostic info. |
| src/coreclr/binder/inc/applicationcontext.inl | Forwards diagnostic info into failure cache add. |
| src/coreclr/binder/inc/defaultassemblybinder.h | Extends BindUsingAssemblyName/worker signatures with optional diagnostic info. |
| src/coreclr/binder/defaultassemblybinder.cpp | Propagates diagnostic info through default binder bind calls. |
| src/coreclr/binder/inc/customassemblybinder.h | Extends BindUsingAssemblyName/worker signatures with optional diagnostic info. |
| src/coreclr/binder/customassemblybinder.cpp | Propagates diagnostic info through custom binder bind calls. |
| if (pDiagnosticInfo != NULL) | ||
| { | ||
| StackSString format; | ||
| format.LoadResource(IDS_BINDING_FAILED_TO_OPEN_FILE); | ||
| SString pathStr(wszAssemblyPath); | ||
| StackSString hrMsg; | ||
| GetHRMsg(hr, hrMsg); | ||
| pDiagnosticInfo->AppendPrintf(format.GetUTF8(), pathStr.GetUTF8(), hrMsg.GetUTF8()); | ||
| } |
There was a problem hiding this comment.
When multiple probing attempts contribute diagnostics (e.g., bundle probe then TPA probe), this appends messages back-to-back with no delimiter, making FusionLog hard to read. Consider inserting a newline (or other clear separator) when pDiagnosticInfo is non-empty before appending the next formatted message.
| StackSString format; | ||
| format.LoadResource(IDS_BINDING_FAILED_TO_INIT_ASSEMBLY); | ||
| StackSString hrMsg; | ||
| GetHRMsg(hr, hrMsg); |
There was a problem hiding this comment.
This appends the init-failure diagnostic directly onto any existing diagnostic text. Since the same SString can be passed through multiple bind attempts/probes, messages can run together. Consider adding a newline/separator when pDiagnosticInfo is non-empty before appending this message (and likewise for other diagnostic append sites).
| GetHRMsg(hr, hrMsg); | |
| GetHRMsg(hr, hrMsg); | |
| if (!pDiagnosticInfo->IsEmpty()) | |
| { | |
| pDiagnosticInfo->Append(W("\n")); | |
| } |
| if (pDiagnosticInfo != NULL && !pFailureCachEntry->GetDiagnosticInfo().IsEmpty()) | ||
| { | ||
| StackSString format; | ||
| format.LoadResource(IDS_BINDING_CACHED_FAILURE_PREFIX); | ||
| pDiagnosticInfo->AppendPrintf(format.GetUTF8(), pFailureCachEntry->GetDiagnosticInfo().GetUTF8()); | ||
| } |
There was a problem hiding this comment.
FailureCache::Lookup appends the cached-failure prefix onto the provided diagnostic buffer without adding a separator. If the caller reuses the same SString across retries or already has diagnostic content, this will produce a run-on FusionLog. Consider prepending a newline/separator when pDiagnosticInfo is non-empty before appending the cached failure message.
Get-only auto-properties can be set from constructors even across partial class files, so no setter is needed. This preserves the readonly backing field semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
39ee0e5 to
7976c56
Compare
src/coreclr/System.Private.CoreLib/src/System/IO/FileNotFoundException.CoreCLR.cs
Show resolved
Hide resolved
- Make the diagnosticInfo constructor delegate to the base constructor instead of duplicating its body. - Remove the unnecessary IsEmpty check at the throw site in AppDomain::BindAssemblySpec — always pass bindDiagnosticInfo since CreateThrowable already gates on m_diagnosticInfo.IsEmpty(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When multiple probing attempts contribute diagnostics (e.g., bundle probe then TPA probe, or cached failure replay), insert a newline before appending each new message so FusionLog is readable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Capture the diagnostic info right after pAssembly->Init fails instead of in the shared Exit path. This makes the error attribution clearer and avoids the else-if coupling with the IsFileNotFound normalization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c1416b4 to
bdc3496
Compare
src/libraries/System.Runtime.Loader/tests/TpaLoadFailureTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.Loader/tests/TpaLoadFailureTest.cs
Outdated
Show resolved
Hide resolved
The Missing and Corrupt assemblies are deleted by the RemoveBindFailureTestAssemblies MSBuild target after build, not by Private=false. Update comments to reflect this. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Note
This PR was generated with the assistance of GitHub Copilot.
Summary
Today, when a TPA assembly fails to load (file locked, corrupt, missing), the exception says only
Could not load file or assembly 'X'. The system cannot find the file specified.— with no indication of the actual file path, the step that failed, or the original error code.This PR threads diagnostic context through the assembly binding pipeline and populates
FileNotFoundException.FusionLogwith actionable information.Changes
Native binder — collect diagnostic info on failure
BinderAcquirePEImage: capture file-open errors and exception messages instead of discarding them viaEX_CATCH_HRESULTGetAssembly: annotate metadata initialization failuresSString* pDiagnosticInfothrough the bind chain:BindAssembly→BindByName→BindLocked→BindByTpaList→GetAssemblymscorrc.rcresourcesGetHRMsgto include human-readable HRESULT descriptionsTryOpenFile: cacheGetLastError()in a local to avoid double-callFailure cache — preserve diagnostic context
FailureCacheEntrynow stores diagnostic info alongside the HRESULT[cached failure]Native VM — thread diagnostics to managed exception
EEFileLoadException: newm_diagnosticInfofield, constructor overload, andThrowoverloadCreateThrowable: passes diagnostic string throughFileLoadException.CreatebridgeManaged — populate
FusionLogFileNotFoundException.FusionLog: changed from{ get; }to{ get; internal set; }diagnosticInfoparameterFileLoadException.Create: acceptschar* pDiagnosticInfo, passes toFileNotFoundExceptionforFileNotFoundkindFull call chain wiring
AssemblyBinder::BindAssemblyByName→BindUsingAssemblyName(virtual) →DefaultAssemblyBinder/CustomAssemblyBinder→AssemblySpec::Bind→AppDomain::BindAssemblySpecEEFileLoadException::ThrowsiteTests
TpaLoadFailureTestwith three scenarios using dedicated TPA test assemblies:COR_E_FILENOTFOUNDERROR_SHARING_VIOLATIONCOR_E_ASSEMBLYEXPECTEDDesign
FAILED(hr)pathsSString*parameters default toNULL; only written on failureGetHRMsgfor HRESULT descriptions,mscorrc.rcfor format strings