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

Packaged release bootstrapper fails on mono #1901

Closed
glopesdev opened this issue Jul 11, 2024 · 5 comments · Fixed by #1902
Closed

Packaged release bootstrapper fails on mono #1901

glopesdev opened this issue Jul 11, 2024 · 5 comments · Fixed by #1902
Labels
bug Something isn't working critical This is a blocking issue affecting all users
Milestone

Comments

@glopesdev
Copy link
Member

glopesdev commented Jul 11, 2024

Running the latest packaged release of the bootstrapper on Linux mono fails with the below stack trace. Compiling the project from source with dotnet build is successful however, so the suspicion is this might be possibly related to ILRepack and embedding of System.Resources.Extensions.

The current workaround is to build from source.

Unhandled Exception:
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentException: This .resources file should not be read with this reader. The resource reader type is "System.Resources.Extensions.DeserializingResourceReader".
Parameter name: reader
  at System.Resources.Extensions.RuntimeResourceSet..ctor (System.Resources.IResourceReader reader) [0x0002f] in <efe718142b824e97b598670ead64f4bf>:0 
  at (wrapper managed-to-native) System.Reflection.RuntimeConstructorInfo.InternalInvoke(System.Reflection.RuntimeConstructorInfo,object,object[],System.Exception&)
  at System.Reflection.RuntimeConstructorInfo.InternalInvoke (System.Object obj, System.Object[] parameters, System.Boolean wrapExceptions) [0x00005] in <d636f104d58046fd9b195699bcb1a744>:0 
   --- End of inner exception stack trace ---
  at System.Reflection.RuntimeConstructorInfo.InternalInvoke (System.Object obj, System.Object[] parameters, System.Boolean wrapExceptions) [0x0001a] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Reflection.RuntimeConstructorInfo.DoInvoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00086] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Reflection.RuntimeConstructorInfo.Invoke (System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.RuntimeType.CreateInstanceImpl (System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, System.Object[] args, System.Globalization.CultureInfo culture, System.Object[] activationAttributes, System.Threading.StackCrawlMark& stackMark) [0x0022b] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Activator.CreateInstance (System.Type type, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder binder, System.Object[] args, System.Globalization.CultureInfo culture, System.Object[] activationAttributes) [0x0009c] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ManifestBasedResourceGroveler.CreateResourceSet (System.IO.Stream store, System.Reflection.Assembly assembly) [0x00142] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ManifestBasedResourceGroveler.GrovelForResourceSet (System.Globalization.CultureInfo culture, System.Collections.Generic.Dictionary`2[TKey,TValue] localResourceSets, System.Boolean tryParents, System.Boolean createIfNotExists, System.Threading.StackCrawlMark& stackMark) [0x000be] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ResourceManager.InternalGetResourceSet (System.Globalization.CultureInfo requestedCulture, System.Boolean createIfNotExists, System.Boolean tryParents, System.Threading.StackCrawlMark& stackMark) [0x00099] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ResourceManager.InternalGetResourceSet (System.Globalization.CultureInfo culture, System.Boolean createIfNotExists, System.Boolean tryParents) [0x00002] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ResourceManager.GetObject (System.String name, System.Globalization.CultureInfo culture, System.Boolean wrapUnmanagedMemStream) [0x00071] in <d636f104d58046fd9b195699bcb1a744>:0 
  at System.Resources.ResourceManager.GetObject (System.String name, System.Globalization.CultureInfo culture) [0x00000] in <d636f104d58046fd9b195699bcb1a744>:0 
  at Bonsai.Editor.Properties.Resources.get_Icon () [0x00005] in <e311ee915bd1459ea82d15dfe13f661c>:0 
  at Bonsai.Editor.StartScreen.InitializeComponent () [0x00a69] in <e311ee915bd1459ea82d15dfe13f661c>:0 
  at Bonsai.Editor.StartScreen..ctor () [0x000a0] in <e311ee915bd1459ea82d15dfe13f661c>:0 
  at (wrapper remoting-invoke-with-check) Bonsai.Editor.StartScreen..ctor()
  at Bonsai.Launcher.LaunchStartScreen (System.String& initialFileName) [0x00005] in <8f3198ce22c344b693cd8ddade06e4bc>:0 
  at Bonsai.Program.Main (System.String[] args) [0x002c7] in <8f3198ce22c344b693cd8ddade06e4bc>:0
@glopesdev glopesdev added bug Something isn't working critical This is a blocking issue affecting all users labels Jul 11, 2024
@PathogenDavid
Copy link
Member

It's somewhat surprising you get to the StartScreen before this goes bang. Were all the packages already restored? Does the bootstrap UI work fine before that?

@glopesdev
Copy link
Member Author

glopesdev commented Jul 11, 2024

Yup, everything works fine with the bootstrap UI, the start screen and main editor. The build kind title text also displays correctly which shows that we are loading the right assembly version.

I mentioned this elsewhere but for completion here I also tested running the repackaged binary built on Linux on a Windows environment and it works fine there too, which suggests there might be a route to get a single repackaged binary that works cross-platform.

@PathogenDavid
Copy link
Member

PathogenDavid commented Jul 12, 2024

I mentioned this elsewhere but for completion here I also tested running the repackaged binary built on Linux on a Windows environment and it works fine there too, which suggests there might be a route to get a single repackaged binary that works cross-platform.

This was actually a red herring. The real difference is the presence of Bonsai.exe.config, which we don't distribute.

In a more typical .NET app it contains a binding redirect for System.Resources.Extensions:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <startup>
    <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.7.2" />
  </startup>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Resources.Extensions" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-8.0.0.0" newVersion="8.0.0.0" />
      </dependentAssembly>
    </assemblyBinding>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
      <dependentAssembly>
        <assemblyIdentity name="System.Runtime.CompilerServices.Unsafe" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-6.0.0.0" newVersion="6.0.0.0" />
      </dependentAssembly>
    </assemblyBinding>
  </runtime>
</configuration>

This is done because the type reference embedded in resources is hard-coded to 4.0.0.0, and the resource loader extension junk relies on that to redirect the type reference to the appropriate assembly at runtime.

For Bonsai I'm relying on our AssemblyResolver handler to perform this redirection. Unfortunately this doesn't take for Mono for some reason. The assembly is resolved and redirected appropriately, but for some reason the resource groveler isn't actually using it. Might be a Mono bug, need to dig further.


Easiest workaround for now is to distribute Bonsai.exe.config, not sure how we feel about that. Could also automatically write it out and restart ourselves if we detect we're running on Mono (along with a self-ignoring .gitignore so people don't start committing it by mistake.)

We could also try to establish a best practice of ignoring the entire Bonsai workspace directory and explicitly un-ignoring files like Bonsai.config. (We could also make such a .gitignore part of the Bonsai environment template.)

@PathogenDavid
Copy link
Member

PathogenDavid commented Jul 12, 2024

TL;DR: Fixed it #1902


System.ArgumentException: This .resources file should not be read with this reader. The resource reader type is "System.Resources.Extensions.DeserializingResourceReader".
Parameter name: reader
  at System.Resources.Extensions.RuntimeResourceSet..ctor (System.Resources.IResourceReader reader) [0x0002f] in <efe718142b824e97b598670ead64f4bf>:0 

So this exception comes from here: RuntimeResourceSet.cs:200

Emphasis on the System.Resources.Extensions.RuntimeResourceSet part. It's actually using the modern resource RuntimeResourceSet, but the RuntimeReosurceSet thinks it was given a reader other than DeserializingResourceReader...but the exception says that's what it was given.

Turns out that Mono loads the assembly more than once, and that the .NET Runtime (Mono or otherwise) loads a new assembly every time you call the Assembly.Load(byte[]) overload specifically.

(This is something that has burned Bonsai before it turns out: a0b3f9e)

Basically we were trying to use DeserializingResourceReader from System.Resources.Extensions[Load A] with RuntimeResourceSet from System.Resources.Extensions[Load B], but the latter was very sensibly expecting a DeserializingResourceReader from [Load B].

Fix was pretty simple, just have to cache the result: #1902

PathogenDavid added a commit to PathogenDavid/bonsai that referenced this issue Jul 12, 2024
Also added copying the NuGet.config to the repack output directory for ease of testing.

Fixes bonsai-rx#1901
@PathogenDavid
Copy link
Member

PathogenDavid commented Jul 12, 2024

As an aside I'm not totally sure why the binding redirect fixes it. Mono actually has observably different load behavior compared to .NET Framework in general.

For example, the following prints True on .NET Framework with one "Resolving..." message, Mono prints False along with two "Resolving..." messages:

AppDomain.CurrentDomain.AssemblyResolve += (_, args) =>
{
    if (args.Name != "Whatever")
        return null;
    byte[] assemblyBytes;
    using (Stream s = File.OpenRead("ClassLibrary1.dll"))
    {
        assemblyBytes = new byte[s.Length];
        int readLength = s.Read(assemblyBytes, 0, assemblyBytes.Length);
        Debug.Assert(readLength == assemblyBytes.Length);
    }

    Assembly result = Assembly.Load(assemblyBytes); ;
    Console.WriteLine($"Resolving '{args.Name}' to {result.FullName}");
    return result;
};

Assembly a1 = Assembly.Load("Whatever");
Assembly a2 = Assembly.Load("Whatever");
Console.WriteLine(ReferenceEquals(a1, a2));

I imagine that the binding redirect (perhaps combined with the strong name) causes it to not do the redundant resolve for some reason.

@glopesdev glopesdev added this to the 2.8.5 milestone Jul 12, 2024
PathogenDavid added a commit to PathogenDavid/bonsai that referenced this issue Jul 13, 2024
Also added copying the NuGet.config to the repack output directory for ease of testing.

Fixes bonsai-rx#1901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical This is a blocking issue affecting all users
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants