Keep the static field dump alive when a type initializer fails - #978
Open
M-r-A wants to merge 1 commit into
Open
Keep the static field dump alive when a type initializer fails#978M-r-A wants to merge 1 commit into
M-r-A wants to merge 1 commit into
Conversation
The Print static fields debug action produced no output at all when any loaded assembly had a static field whose declaring type's initializer throws. Reading a static field runs that initializer, which is arbitrary mod code, and FieldValue called field.GetValue(null) unguarded, so the exception unwound out of the action and Dialog_Debug reported a window failure instead of the listing. The action exists to surface unsynchronized mutable static state, which is a common cause of desyncs, so it is reached for precisely when something is already wrong and odd inputs are most likely. TryReadStaticValue now catches per field and reports the base exception's type name in place of the value; the outer TypeInitializationException names the mechanism and hides the cause. TypesOf falls back to the non-null entries of ReflectionTypeLoadException.Types, so an assembly referencing something absent still dumps the types it did resolve. Catching Exception rather than a specific type is deliberate: the thrown type is whatever a mod's initializer happens to throw, and GetValue can fail without an initializer being involved. Catching too much costs one line reading [unreadable: X]; catching too little costs the entire report. The two fragile reflection calls live in Common because Source/Tests cannot reference Client, which needs the game's assemblies to load. No behaviour beyond the guards changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mods -> Print static fields (mods)produces no output at all when anyloaded assembly has a static field whose declaring type's initializer throws.
Root cause
Reading a static field runs that type's initializer, and that is arbitrary mod
code.
FieldValuecalledfield.GetValue(null)unguarded, so the exceptionunwound out of the debug action and
Dialog_Debugreported a window failureinstead of the listing.
One bad initializer anywhere in the loaded assemblies costs the entire report.
That matters more than it first appears, because of when this action gets used:
it exists to surface unsynchronized mutable static state, which is one of the
first things reached for when a session desyncs. It is invoked precisely when
something is already wrong — which is when odd inputs are most likely.
What changed
TryReadStaticValuecatches per field and reports the base exception'stype name in place of the value. The outer exception is always
TypeInitializationException, which names the mechanism and hides the cause;the inner one is what tells a reader whether to care.
TypesOffalls back toe.Types.Where(t => t != null)onReflectionTypeLoadException, so an assembly referencing something absentstill dumps the types it did resolve.
catch (Exception)rather than a specific type is deliberate: the thrown type iswhatever a mod's initializer happens to throw, and
GetValuecan also failwithout an initializer being involved. The cost of catching too much is one line
reading
[unreadable: X]. The cost of catching too little is losing all 1,614 ofthem.
Why the reflection moved to
CommonSource/Testsreferences onlyChatCommandContracts,CommonandSourceGen—not
Client, which needs the game's assemblies to load and throwsFileNotFoundException: Could not load file or assembly 'Assembly-CSharp'in CI.The two fragile calls therefore live in
Source/Common/StaticFieldDump.cs, andthe debug action calls them. No behaviour beyond the guards changed.
Testing
Unit: 160 pass (158 before, 2 added).
The added test reproduces the observed chain
TypeInitializationException ---> DllNotFoundException: libc. Its stand-in typeuses an explicit static constructor to suppress
beforefieldinit, soinitialization happens exactly on first read rather than at a moment of the
runtime's choosing. The declaration needs no native library, so the test behaves
identically on every platform.
Manual, main menu, dev mode:
[unreadable: …]entries and no exception — 15DllNotFoundExceptionfrom MonoMod's foreign-platform interop, and 1NullReferenceExceptionfrom a content mod whose initializer reads DLC datawith the DLC inactive. Either alone previously cost the whole report.
Print static fields (game)unchanged: 2,247 lines, zero unreadable.Note for review
This is not a Windows-specific fix. MonoMod ships the Windows, Linux and macOS
backends in one assembly and the dump walks all of them, so the same defect
occurs on every platform — only the type names in the stack differ.