Skip to content

[mono] Class constructor for beforefieldinit class runs even if static fields are not accessed #88066

Open

Description

@akurone

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Type layout below causes System.TypeInitializationException to be thrown in WASM (same code can run in a server-type project):

public sealed class TwistedType {
  public static IEnumerable<TwistedType> TheTroubleMaker = NestedStatic.ArrayOfTwistedType.Prepend(new());

  public static class NestedStatic {
    public static TwistedType[] ArrayOfTwistedType = { new() };
  }

  public string World { get; } = "World";
}

The relevant member of NestedStatic type is referenced like:

<h1>Hello, @TwistedType.NestedStatic.ArrayOfTwistedType[0].World!</h1>

Expected Behavior

Type initialization should be successful in WASM as in server.

Steps To Reproduce

Clone this repo and run.
Versions:

  • Chrome: 116.0.5829.0
  • .net: 7.0.304

Exceptions (if any)

Microsoft.AspNetCore.Components.WebAssembly.Rendering.WebAssemblyRenderer[100]
      Unhandled exception rendering component: The type initializer for 'NestedStatic' threw an exception.
System.TypeInitializationException: The type initializer for 'NestedStatic' threw an exception.
 ---> System.TypeInitializationException: The type initializer for 'BlzWasmTypeInitRepro.TwistedType' threw an exception.
 ---> System.ArgumentNullException: Value cannot be null. (Parameter 'source')
   at System.Linq.ThrowHelper.ThrowArgumentNullException(ExceptionArgument argument)
   at System.Linq.Enumerable.Prepend[TwistedType](IEnumerable`1 source, TwistedType element)
   at BlzWasmTypeInitRepro.TwistedType..cctor() in BlzWasmTypeInitRepro\Types.cs:line 5
   --- End of inner exception stack trace ---
   at BlzWasmTypeInitRepro.TwistedType.NestedStatic..cctor() in BlzWasmTypeInitRepro\Types.cs:line 9
   --- End of inner exception stack trace ---
   at BlzWasmTypeInitRepro.Pages.Index.BuildRenderTree(RenderTreeBuilder __builder) in BlzWasmTypeInitRepro\Pages\Index.razor:line 3
   at Microsoft.AspNetCore.Components.ComponentBase.<.ctor>b__6_0(RenderTreeBuilder builder)
   at Microsoft.AspNetCore.Components.Rendering.ComponentState.RenderIntoBatch(RenderBatchBuilder batchBuilder, RenderFragment renderFragment, Exception& renderFragmentException)

.NET Version

7.0.304

Anything else?

No response

Activity

  1. transferred this issue fromdotnet/aspnetcoreon Jun 26, 2023
  2. ghost added
    needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
    on Jun 26, 2023
  3. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Jun 26, 2023
  4. removed
    needs-area-labelAn area label is needed to ensure this gets routed to the appropriate area owners
    on Jun 28, 2023
  5. removed
    untriagedNew issue has not been triaged by the area owner
    on Jul 24, 2023
  6. added this to the 8.0.0 milestone on Jul 24, 2023
  7. marek-safar commented on Jul 24, 2023

    @marek-safar
    Contributor
  8. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    Repros in console app using mono as the runtime:

    using System;
    using System.Linq;
    using System.Collections.Generic;
    
    Console.WriteLine ($"Hello {TwistedType.NestedStatic.ArrayOfTwistedType[0].World}");
    
    public sealed class TwistedType {
      public static IEnumerable<TwistedType> TheTroubleMaker = NestedStatic.ArrayOfTwistedType.Prepend(new());
    
      public static class NestedStatic {
        public static TwistedType[] ArrayOfTwistedType = { new() };
      }
    
      public string World { get; } = "World";
    }
    $ dotnet run --self-contained -p:UseMonoRuntime=true -r osx-arm64

    At first glance, I think this might be allowed by ECMA you cannot depend on the order of execution for static constructors...

  9. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    CoreCLR is also quite fragile here. change #if true to #if false below to see a difference - the first one runs, the second one throws

    using System;
    using System.Collections.Generic;
    
    Console.WriteLine ($"Hello {SecondOne.ArrayOfFirstOne[0].World}");
    
    
    public static class Helper
    {
        public static FirstOne[] Prepend(FirstOne[] coll, FirstOne oneMore)
        {
            var copy = new FirstOne[coll.Length + 1];
            copy[0] = oneMore;
            for (int i = 0; i < coll.Length; ++i)
                copy[i+1] = coll[i];
            return copy;
        }
    }
    
    public sealed class FirstOne {
    #if true
        // this works
        
        public static IEnumerable<FirstOne> TheTroubleMaker = Helper.Prepend (SecondOne.ArrayOfFirstOne, new());
    
    #else
        // this throws
        
        public static IEnumerable<FirstOne> TheTroubleMaker;
    
        static FirstOne() {
            //Console.WriteLine ("FirstOne .cctor");
            TheTroubleMaker = Helper.Prepend (SecondOne.ArrayOfFirstOne, new());
        }
    #endif
    
        public string World { get; } = "World";
    }
    
    
    public static class SecondOne {
        public static FirstOne[] ArrayOfFirstOne = { new () };
    
        //static SecondOne() {
        //    Console.WriteLine ("SecondOne .cctor");
        //    ArrayOfFirstOne = new FirstOne[] { new() };
        //}
    }
  10. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    Hmm... comparing the IL for the working and broken versions of FirstOne, it looks like the working version has beforefieldinit and the broken one doesn't.

  11. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    Ok, this isn't one of the cases where ECMA allows us some leeway because of circular dependencies. Rather, this is a case where the cctor must run "at or before static field access" and mono is leaning on the "or before" (specifically I think we run it when the instance constructor for FirstOne is called)

    It looks like mono is calling a beforefieldinit static constructor even if we never touch the static fields of that class

    using System;
    using System.Runtime.CompilerServices;
    using System.Collections.Generic;
    
    object o  = SecondOne.ArrayOfFirstOne[0];
    Console.WriteLine ($"Hello {o!=null}");
    
    public static class Helper
    {
        [MethodImpl(MethodImplOptions.NoInlining)]
        public static FirstOne[] PrependOne(FirstOne[] coll)
        {
            throw new Exception ("Called");
        }
    }
    
    public sealed class FirstOne {
        public static IEnumerable<FirstOne> TheTroubleMaker = Helper.PrependOne (SecondOne.ArrayOfFirstOne);
    
        public FirstOne() {}
    
        public string World() => "World";
    }
    
    public static class SecondOne {
        public static FirstOne[] ArrayOfFirstOne = { new () };
    }

    It looks like we're running the cctor for FirstOne when SecondOne tries to instantiate it. But constructor invocation for a beforefieldinit class is not sufficient. Only static field access (or explicit cctor invocation) should trigger it, I think.

    There is a variant where instead of calling the instance constructor, we call a static method from FirstOne. In this case, too, CoreCLR doesn't call the cctor for FirstOne

    Details
    using System;
    using System.Runtime.CompilerServices;
    using System.Collections.Generic;
    

    object o = SecondOne.ArrayOfFirstOne[0];
    Console.WriteLine ($"Hello {o!=null}");

    public static class Helper
    {
    [MethodImpl(MethodImplOptions.NoInlining)]
    public static FirstOne[] PrependOne(FirstOne[] coll)
    {
    throw new Exception ("Called");
    }
    }

    public sealed class FirstOne {
    public static IEnumerable TheTroubleMaker = Helper.PrependOne (SecondOne.ArrayOfFirstOne);

    public FirstOne() {}
    
    public string World() => "World";
    
    public static FirstOne SomeStaticMethod() { return null; }
    

    }

    public static class SecondOne {
    public static FirstOne[] ArrayOfFirstOne = { FirstOne.SomeStaticMethod () };
    }

  12. 1 remaining item

  13. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    @vargaz @BrzVlad I see some calls to mono_class_is_before_field_init in method-to-ir, so we're aware that sometimes beforefieldinit means we shouldn't run the cctor. But it looks like we do it anyway. In the interpreter I don't see anything that looks at beforefieldinit, so it's probably too eager, also.

  14. vargaz commented on Jul 24, 2023

    @vargaz
    Contributor

    Probably a dup of:
    #77513

  15. lambdageek commented on Jul 24, 2023

    @lambdageek
    Member

    I think #77513 is not quite the same. In that one, the bad static field is used in the body of a called method - it's just not on an executed path. In this one, the bad static field is not used at all in any JITted method.
    Fixing 77513 seems harder

  16. LeVladIonescu commented on Aug 14, 2023

    @LeVladIonescu
    Contributor

    Moving this to Postponed to .Net9 with high priority.

  17. modified the milestones: 8.0.0, 9.0.0 on Aug 14, 2023
  18. added
    JitUntriagedCLR JIT issues needing additional triage
    needs-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration
    on Feb 9, 2024
  19. modified the milestones: 9.0.0, Future on Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

JitUntriagedCLR JIT issues needing additional triagearea-Codegen-meta-mononeeds-further-triageIssue has been initially triaged, but needs deeper consideration or reconsideration

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions