Skip to content
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.

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

HarmonyLib dependent must not be an exported type #9

Closed
drok opened this issue Mar 15, 2021 · 4 comments
Closed

HarmonyLib dependent must not be an exported type #9

drok opened this issue Mar 15, 2021 · 4 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@drok
Copy link
Owner

drok commented Mar 15, 2021

Classes that use HarmonyLib features should not be public, because this causes ReflectionTypeLoadException errors and TypeLoadException errors.

Failure mechanism

The game loads mods in 3 phases:

  1. Scan all assemblies, note their assemblyName and directory.
  2. Load mod assemblies one mod at a time, starting with the dependency tree leaves.
  3. Instantiate enabled assemblies, and OnEnable() them
  4. further use.

The Harmony Mod assemblies may be loaded at phase 2, but some time after the mod that uses them. This should not matter, because no mods are instantiated until all the assemblies are loaded. If a dependent assembly is a perfect match (ie, matching public key token and culture), it is "borrowed" from the mod that supplies it, to load it into the mod that requires it. The borrowing only happens once, because once loaded into memory, it is used for all mods that need it. If two mods need the same dependency, that dependency will appear on the assembly list of the mod that happened the need it first, and no longer on the assembly list of the supplying mod.

If the match is not perfect (ie, either public key token or culture don't match), then the game cannot load the needed assembly as a leaf in the dependency tree of the mod that needs it. Instead, it would be loaded as a leaf in the dependency tree of the mod that supplies it. The effect is that by the time phase 3 starts, all needed assemblies are loaded.

However, if a mod with the dependency exposes this dependency through a public class (type), then the public class must be resolved when the game AddAssembly due to GetExportedTypes() - where it searches for an IUserMod type. Since the signed dependency is not yet loaded, GetExportedTypes() throws a TypeLoadException, and the IUserMod class is not found, and will not be instantiated later at phase 3.

In short, to avoid dependency order problems like this, the depended types must be inside internal or private types, so they do not need to be Resolved/loaded at AddAssembly() time.

Practical effects

This bug would cause affected mods to crash with TypeLoadException and be unloadable if a Harmony mod is not subscribed.

The expected behaviour is that mods should not crash, should load, and should either:

  • Automatically subscribe Harmony (function provided by CitiesHarmony.API), or
  • pop up a warning message if Harmony is already subscribed but is not present

How to test if your mod is affected

  1. First Time User Scenario (user forgot to also subscribe the dependency Harmony):
    1. Subscribe both your mod and Harmony.
    2. delete the Harmony mod folder from the workshop folder.
    3. Start the game
    4. Observe crash logged in output_log.txt.
  2. User of local mods. (Epic Games players; content creators who require a stable, un-updated set of mods for production work; Players who run the game offline, on airplanes, for example; generally new players who are unaware of the Harmony workings):
    1. Subscribe your mod.
    2. Copy your mod to local %LOCALAPPDATA%. Do not copy the Harmony folder locally.
    3. Start the game with --noWorkshop.
    4. Observe crash logged in output_log.txt.
  3. Player who switches from boformer's Harmony to the redesigned (this) Harmony (users who want to access the Harmony Report to diagnose their mod collection issue):
    1. Subscribe your mod and boformer's Harmony.
    2. Unsubscribe boformer's Harmony
    3. Subscribe Harmony (redesigned)
    4. Start the game normally.
    5. Observe crash logged in output_log.txt.

Workarounds

Since this bug is effectively a load-ordering bug, it can be worked around by ensuring Harmony is subscribed and loaded before the affected mod.

  1. If used with local mods, Harmony should be in a folder with a name alphabetically earlier than the affected mod's (eg. 000-Harmony-should-load-first). Local mods are loaded in alphabetical order by folder name.
  2. If used with Harmony redesigned, the affected mods should be subscribed after Harmony is subscribed. Workshop items are loaded in the order they were subscribed.

Solutions

See the drok/PopulationDemographics@63d2bd9 and drok/PopulationDemographics@708c894 commits for potential ways to fix this issue.

drok added a commit that referenced this issue Mar 15, 2021
Mods that are missing dependencies throw the type load exception (TLE) when
instantiated or probed by fetching PluginInfo.userModInstance, but the
resulting stack trace does not attribute the fault to the mod that caused
it.

In order to track proper attribution, this commit installs an alternate
getter for userModInstance, which reports the offending mod directly
when the TLEs are thrown on construction.

A mod can still throw TLE during operation, eg, if it attempts to use
a class from another assembly that is missing, but that TLE's stack trace
will contain the loading mod, and thus will be attributed to it.

The Harmony Mod did not handle mods broken this way.

This commit fixes the handling of other mods being unable to load due to
this bug, catches those TLE exceptions, and adds them to the Harmony Report

Fixes issue #9
drok added a commit to drok/PopulationDemographics that referenced this issue Mar 17, 2021
… member

Fixes issue drok/Harmony-CitiesSkylines#9 which causes the Exception below
when the mod is loaded before Harmony, or with Harmony absent.


ReflectionTypeLoadException: The classes in the module cannot be loaded.
  at (wrapper managed-to-native) System.Reflection.Assembly:GetTypes (bool)
  at System.Reflection.Assembly.GetExportedTypes () [0x00000] in <filename unknown>:0
  at ColossalFramework.Plugins.PluginManager+PluginInfo.AddAssembly (System.Reflection.Assembly asm) [0x00000] in <filename unknown>:0
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)
ColossalFramework.Plugins.PluginInfo:AddAssembly(Assembly)
ColossalFramework.Plugins.PluginManager:LoadDependenciesRecursive(Assembly, Dictionary`2, Dictionary`2, List`1)
ColossalFramework.Plugins.PluginManager:LoadDependencies(Dictionary`2, Dictionary`2)
ColossalFramework.Plugins.PluginManager:LoadAssemblies(Dictionary`2)
ColossalFramework.Plugins.PluginManager:LoadPlugins()
Starter:Awake()
drok added a commit to drok/PopulationDemographics that referenced this issue Mar 17, 2021
Fixes issue drok/Harmony-CitiesSkylines#9 which causes the Exception below
when the mod is loaded before Harmony, or with Harmony absent.


ReflectionTypeLoadException: The classes in the module cannot be loaded.
  at (wrapper managed-to-native) System.Reflection.Assembly:GetTypes (bool)
  at System.Reflection.Assembly.GetExportedTypes () [0x00000] in <filename unknown>:0
  at ColossalFramework.Plugins.PluginManager+PluginInfo.AddAssembly (System.Reflection.Assembly asm) [0x00000] in <filename unknown>:0
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)
ColossalFramework.Plugins.PluginInfo:AddAssembly(Assembly)
ColossalFramework.Plugins.PluginManager:LoadDependenciesRecursive(Assembly, Dictionary`2, Dictionary`2, List`1)
ColossalFramework.Plugins.PluginManager:LoadDependencies(Dictionary`2, Dictionary`2)
ColossalFramework.Plugins.PluginManager:LoadAssemblies(Dictionary`2)
ColossalFramework.Plugins.PluginManager:LoadPlugins()
Starter:Awake()
@drok
Copy link
Owner Author

drok commented Mar 17, 2021

Mods that currently contain this bug:

(partial list, there may be others I'm unaware of)

@drok
Copy link
Owner Author

drok commented Mar 18, 2021

Issue is detected and reported on the Harmony Report as of 6d05eff

@drok drok closed this as completed Mar 18, 2021
@drok drok added enhancement New feature or request Fixed documentation Improvements or additions to documentation labels Mar 18, 2021
@drok drok pinned this issue Mar 18, 2021
@ghost
Copy link

ghost commented Mar 20, 2021

Population Demographics and Building Usage have been updated to gracefully handle missing Harmony mod.

@drok
Copy link
Owner Author

drok commented Mar 21, 2021

Population Demographics and Building Usage have been updated to gracefully handle missing Harmony mod.

Thank you for your quick fix, cheers!

drok added a commit that referenced this issue Feb 12, 2022
The tests demonstrate buggy behaviour
These tests verify that the bugs are caught and reported
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant