Include requesting assembly chain in assembly load failure exceptions#125795
Include requesting assembly chain in assembly load failure exceptions#125795
Conversation
When an assembly fails to load, the FileLoadException/FileNotFoundException now includes the name of the requesting (parent) assembly in the FusionLog property. This helps diagnose dependency chain issues by showing which assembly triggered the failed load. The information flows from the native AssemblySpec::GetParentAssembly() through EEFileLoadException::CreateThrowable() to the managed exception's FusionLog property, which is displayed in ToString() output. Fixes #9185 Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
BadImageFormatException is also created by EEFileLoadException::CreateThrowable() when the HRESULT maps to kBadImageFormatException. Without the 3-arg constructor, the requesting assembly info would be silently dropped for bad image format errors. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
|
@copilot we should include the full requesting assembly chain instead of just the parent. |
Walk the inner exception chain to build the full dependency path. When A→B→C→D and D fails, D's FusionLog now shows: Requesting assembly: 'C' Requesting assembly: 'B' Requesting assembly: 'A' At each Throw(AssemblySpec*) level, the current parent assembly name is appended to every inner EEFileLoadException's chain. The managed FormatRequestingAssemblyChain helper formats each newline-separated name with the "Requesting assembly:" resource string. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
Updated in 228155e. Now at each The managed |
…dd tests The requesting assembly chain is now appended to the exception's Message property instead of FusionLog. This means users see the chain directly in the exception message when it is thrown, without needing to inspect FusionLog separately. Added tests for all three exception types (FileLoadException, FileNotFoundException, BadImageFormatException) that verify: - Single requesting assembly appears in Message - Multiple requesting assemblies (chain) appear in Message - FusionLog remains null Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
…missing transitive dependency Remove the 6 reflection-based tests that invoked private constructors via reflection from BadImageFormatExceptionTests, FileLoadExceptionTests, and FileNotFoundExceptionTests. Add an integration test in System.Runtime.Loader.Tests that creates a real dependency chain (Root -> Mid -> Leaf) where the transitive dependency (Leaf) is missing at runtime. The test verifies that a FileNotFoundException is thrown with the correct missing assembly name when the chain is exercised. Three helper assembly projects are added: - MissingDependency.Leaf (not deployed via PrivateAssets=all) - MissingDependency.Mid (depends on Leaf) - MissingDependency.Root (depends on Mid) Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
The test now asserts that the FileNotFoundException message contains "MissingDependency.Mid" (the requesting assembly), verifying that the entire dependency chain is reported in the exception message, not just the missing assembly file name. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
The test now asserts that both MissingDependency.Mid and MissingDependency.Root appear in the exception message, verifying the entire dependency chain (Root → Mid → Leaf) is reported. Co-authored-by: elinor-fung <47805090+elinor-fung@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
… cache The binding cache stores raw Assembly* parent pointers via CopyFrom when assemblies are bound. If a collectible AssemblyLoadContext is unloaded, the parent assembly may be freed while the cache entry persists — the cache cleanup only removes entries where m_pAssembly is the collectible assembly, not entries where m_pParentAssembly points to it. LookupParentAssemblyForAssembly iterates all cache entries and could return a stale parent pointer, causing a SIGSEGV when the chain-walking code in EEFileLoadException::Throw dereferences it via GetDisplayName. Add a ContainsAssembly validation before returning a parent, so stale entries are skipped and iteration continues to find a valid parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This sounds like a problem for workloads with thousands of assemblies (not unusual). |
When a collectible ALC is unloaded, RemoveAssembly removes entries where m_pAssembly matches the unloaded assembly, but entries that reference it as m_pParentAssembly were left with stale pointers. LookupParentAssemblyForAssembly could then return a freed pointer during the chain walk in EEFileLoadException::Throw. Fix the root cause by also clearing m_pParentAssembly on any cache entry that references the unloaded assembly as its parent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Build the chain string under DomainCacheCrst so all Assembly* dereferences are safe from concurrent collectible ALC unload. GetParentAssemblyChain on AppDomain acquires the lock, scans the binding cache once into a MapSHash<Assembly*, Assembly*>, then walks the map to build the chain string — all under the lock. Remove now-unused FindCachedParentAssembly and LookupParentAssemblyForAssembly. Also rename test to MissingTransitiveDependency_ShowsRequestingAssemblyChain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @@ -77,6 +78,9 @@ public override string ToString() | |||
| if (!string.IsNullOrEmpty(FileName)) | |||
| s += Environment.NewLineConst + SR.Format(SR.IO_FileName_Name, FileName); | |||
|
|
|||
| if (!string.IsNullOrEmpty(_requestingAssemblyChain)) | |||
| s += Environment.NewLineConst + SR.Format(SR.IO_FileLoad_RequestedBy, _requestingAssemblyChain.ReplaceLineEndings()); | |||
|
|
|||
There was a problem hiding this comment.
The new _requestingAssemblyChain field is appended in ToString(), but it isn’t preserved across exception serialization/deserialization (the SerializationInfo ctor and GetObjectData only round-trip FileName/FusionLog). Since this type is still [Serializable] and already maintains custom serialization fields, please include the requesting assembly chain in the serialization payload and restore it in the serialization ctor so diagnostics survive remoting/logging scenarios that serialize exceptions.
This comment has been minimized.
This comment has been minimized.
This contradicts what we have just done in #126428 . We should be consistent - either keep updating the binary serializers or not at all. |
This comment has been minimized.
This comment has been minimized.
Restore the requesting assembly chain across legacy exception serialization round-trips for FileLoadException, FileNotFoundException, and BadImageFormatException, keeping the behavior consistent with other updated binary serializer paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| { | ||
| FileName = info.GetString("FileNotFound_FileName"); | ||
| FusionLog = info.GetString("FileNotFound_FusionLog"); | ||
| _requestingAssemblyChain = info.GetString("FileNotFound_RequestingAssemblyChain"); |
There was a problem hiding this comment.
The serialization constructor now unconditionally reads "FileNotFound_RequestingAssemblyChain" via SerializationInfo.GetString. If this exception is deserialized from older serialized data that doesn't contain this entry, GetString will throw a SerializationException. Consider treating this value as optional (e.g., catch SerializationException / check for the entry) and default to null when missing for better forward/backward compatibility.
| _requestingAssemblyChain = info.GetString("FileNotFound_RequestingAssemblyChain"); | |
| try | |
| { | |
| _requestingAssemblyChain = info.GetString("FileNotFound_RequestingAssemblyChain"); | |
| } | |
| catch (SerializationException) | |
| { | |
| _requestingAssemblyChain = null; | |
| } |
| protected FileLoadException(SerializationInfo info, StreamingContext context) | ||
| : base(info, context) | ||
| { | ||
| FileName = info.GetString("FileLoad_FileName"); | ||
| FusionLog = info.GetString("FileLoad_FusionLog"); | ||
| _requestingAssemblyChain = info.GetString("FileLoad_RequestingAssemblyChain"); | ||
| } |
There was a problem hiding this comment.
The serialization constructor now unconditionally reads "FileLoad_RequestingAssemblyChain" via SerializationInfo.GetString. If FileLoadException is deserialized from older serialized data that lacks this entry, GetString will throw. Consider making this field optional during deserialization (default to null when missing).
src/libraries/System.Private.CoreLib/src/System/BadImageFormatException.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Runtime.Loader/tests/AssemblyLoadContextTest.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Use SerializationInfo.GetValueNoThrow for the new *_RequestingAssemblyChain fields so older payloads that do not contain the data still deserialize successfully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…est.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| // Build the requesting assembly chain: start with the immediate parent, | ||
| // then walk up the binding cache to find transitive requesting assemblies. | ||
| StackSString requestingChain; | ||
| pParentAssembly->GetDisplayName(requestingChain); | ||
|
|
||
| EX_TRY | ||
| { | ||
| const int MaxChainDepth = 10; | ||
| AppDomain::GetCurrentDomain()->GetParentAssemblyChain( | ||
| pParentAssembly, requestingChain, MaxChainDepth); | ||
| } | ||
| EX_CATCH | ||
| { | ||
| // If the chain walk fails for any reason, just use what we have so far | ||
| } | ||
| EX_END_CATCH | ||
|
|
||
| pException->SetRequestingAssemblyChain(requestingChain); |
There was a problem hiding this comment.
pParentAssembly->GetDisplayName(requestingChain) (and the initial construction of requestingChain) is outside the EX_TRY/EX_CATCH best-effort block. If GetDisplayName throws (e.g., OOM while formatting the display name), this Throw path can end up throwing that secondary exception instead of the intended EEFileLoadException. Consider wrapping the entire requesting-chain construction (including the initial GetDisplayName and SetRequestingAssemblyChain) in an EX_TRY with EX_SWALLOW_NONTERMINAL (or similar) so diagnostics failures never change the primary failure being thrown.
| // Build the requesting assembly chain: start with the immediate parent, | |
| // then walk up the binding cache to find transitive requesting assemblies. | |
| StackSString requestingChain; | |
| pParentAssembly->GetDisplayName(requestingChain); | |
| EX_TRY | |
| { | |
| const int MaxChainDepth = 10; | |
| AppDomain::GetCurrentDomain()->GetParentAssemblyChain( | |
| pParentAssembly, requestingChain, MaxChainDepth); | |
| } | |
| EX_CATCH | |
| { | |
| // If the chain walk fails for any reason, just use what we have so far | |
| } | |
| EX_END_CATCH | |
| pException->SetRequestingAssemblyChain(requestingChain); | |
| EX_TRY | |
| { | |
| // Build the requesting assembly chain: start with the immediate parent, | |
| // then walk up the binding cache to find transitive requesting assemblies. | |
| StackSString requestingChain; | |
| pParentAssembly->GetDisplayName(requestingChain); | |
| const int MaxChainDepth = 10; | |
| AppDomain::GetCurrentDomain()->GetParentAssemblyChain( | |
| pParentAssembly, requestingChain, MaxChainDepth); | |
| pException->SetRequestingAssemblyChain(requestingChain); | |
| } | |
| EX_CATCH | |
| { | |
| // If building the requesting assembly chain fails for any reason, | |
| // ignore the diagnostic data and preserve the primary exception. | |
| } | |
| EX_END_CATCH |
| Assert.Contains("MissingDependency.Mid", exString); | ||
| Assert.Contains(" --> ", exString); | ||
| Assert.Contains("MissingDependency.Root", exString); |
There was a problem hiding this comment.
The assertions for MissingDependency.Mid / MissingDependency.Root are currently satisfied by the stack trace as well as by the new requesting-assembly-chain text, so this test may still pass even if the new "Requested by" block is missing or incomplete. Consider asserting that these assembly names appear before the stack trace section (e.g., before the first "\n at ") to ensure they’re coming from the requesting chain rather than from stack frames.
| Assert.Contains("MissingDependency.Mid", exString); | |
| Assert.Contains(" --> ", exString); | |
| Assert.Contains("MissingDependency.Root", exString); | |
| int stackTraceIndex = exString.IndexOf(Environment.NewLine + " at ", StringComparison.Ordinal); | |
| string messagePortion = stackTraceIndex >= 0 ? exString[..stackTraceIndex] : exString; | |
| Assert.Contains("MissingDependency.Mid", messagePortion); | |
| Assert.Contains(" --> ", exString); | |
| Assert.Contains("MissingDependency.Root", messagePortion); |
🤖 Copilot Code Review — PR #125795Note This review was generated by GitHub Copilot. Holistic AssessmentMotivation: Well-justified. Issue #9185 (open since 2018) captures a genuine, widespread developer pain point: transitive assembly load failures give no indication of which assembly triggered the load, making Approach: Reasonable overall. Walking the binding cache at throw time is pragmatic and low-overhead. The "best-effort" framing (exceptions swallowed) is appropriate for diagnostic metadata. However there are correctness gaps around how the immediate parent's display name is retrieved (not inside the best-effort guard) and a formatting bug in Summary: Detailed Findings❌
|
This pull request enhances .NET exception diagnostics for assembly loading failures by tracking and surfacing the chain of requesting assemblies when a dependency is missing or invalid. This helps developers more easily identify which assemblies triggered a load failure, especially in complex dependency scenarios.
Exception information:
BadImageFormatException,FileLoadException, andFileNotFoundExceptionnow accept an additionalrequestingAssemblyChainparameter, which records the chain of assemblies that led to the load failure. This chain is stored and included in the exception’s string representation.ToString()methods of these exceptions now append a "Requested by: ..." message listing the requesting assembly chain, improving error clarity for users.IO_FileLoad_RequestedBy, is added for this message.Runtime and binding cache:
AssemblySpecBindingCache. New methods allow walking the chain of requesting assemblies up to a depth of 10, building a diagnostic chain for exceptions.EEFileLoadExceptionand related logic are updated to capture and propagate the requesting assembly chain, including in exception cloning and when thrown from the runtime.Testing and validation:
MissingDependency.Leaf,MissingDependency.Mid) and a test inAssemblyLoadContextTestto verify that the requesting assembly chain appears in exception messages when a transitive dependency is missing.These changes significantly improve the developer experience when diagnosing assembly loading failures by providing more actionable information in exception messages.
Example:
@dotnet/appmodel @AaronRobinsonMSFT
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.