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

[hot reload][coreclr] cumulative updates to properties in generic classes crash the runtme. #85605

Closed
lambdageek opened this issue May 1, 2023 · 5 comments · Fixed by #85655

Comments

@lambdageek
Copy link
Member

lambdageek commented May 1, 2023

Applying cumulative updates to properties in generic classes seems to shutdown coreclr.

In effect this is the code before the first update:

            using System;
            namespace testProperty;
            public class C1<T> {
                public T MyProp1 {get; set;}

                public static int M1() {
                    Console.WriteLine("Hello");
                    return 17;
                }
            }

Then the update changes it to:

                using System;
                namespace testProperty;
                public class C1<T> {
                    public T MyProp1 {get; set;}

                    public T MyProp2 {
                        get => default(T);
                        set {
                            Console.WriteLine($"setting MyProp2 to {value}");
                        }
                    }

                    public static int M1() {
                        var x = new C1<string>();
                        x.MyProp1 = "hello1";
                        x.MyProp2 = "hello2";
                        return 42;
                    }
                }

And finally change it to:

                using System;
                namespace testProperty;
                public class C1<T> {
                    public T MyProp1 {get; set;}

                    public T MyProp2 {
                        get => default(T);
                        set {
                            Console.WriteLine($"updated setting MyProp2 to {value}");
                        }
                    }
                    public T[] MyProp3 {
                        get => default(T[]);
                        set {
                            Console.WriteLine($"setting MyProp3 to {value}");
                        }
                    }

                    public static int M1() {
                        var x = new C1<string>();
                        x.MyProp1 = "hello1";
                        x.MyProp2 = "hello2";
                        x.MyProp3 = new string[] { "hello3" };
                        return 45;
                    }
                }

The actual invocations are done like this (ApplyUpdate is just loading the bytes from the .dmeta and .dil files and calling the System.Reflection.Metadata.MetadataUpdater.ApplyUpdate method):

        int x = testProperty.C1<int>.M1();
        ApplyUpdate();
        x = testProperty.C1<string>.M1();
        if (x != 42) {
            throw new Exception("Expected 42, got " + x);
        }
        PropertyInfo pInfo = typeof(testProperty.C1<string>).GetProperty("MyProp2");
        if (pInfo == null) {
            throw new Exception("Can't find newly added property: MyProp2");
        }
        ApplyUpdate(2);
        Console.WriteLine("Applied second update");
        x = testProperty.C1<string>.M1();
        if (x != 45) {
            throw new Exception("Expected 45, got " + x);
        }
        pInfo = typeof(testProperty.C1<string>).GetProperty("MyProp3");
        if (pInfo == null) {
            throw new Exception("Can't find newly added property: MyProp3");
        }
        MethodInfo mInfo = pInfo.GetGetMethod();
        if (mInfo == null) {
            throw new Exception("Can't find newly added property getter: MyProp3");
        }

I'm including a zip file with a .dll and .dmeta and .dil files with the changes. Use the following runner program to run it after unzipping the test case into some directory.

reproProps.zip

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;

if (args.Length != 1)
{
    Console.WriteLine("Usage: generic-runner <directory>");
    return 1;
}

string directory = args[0];

if (!Directory.Exists(directory))
{
    Console.WriteLine("Directory does not exist: {0}", directory);
    return 1;
}


foreach (var subdir in Directory.GetDirectories(directory))
{
    var alc = System.Runtime.Loader.AssemblyLoadContext.Default;

    var assemblies = Directory.GetFiles(subdir, "*.dll");
    foreach (var assembly in assemblies)
    {
        Console.WriteLine ($">>> {assembly}");
        var asm = alc.LoadFromAssemblyPath(assembly);
        var type = asm.GetType("HotReloadTest", throwOnError: false);
        if (type == null)
            continue;
        var mi = type.GetMethod("Run", BindingFlags.Static | BindingFlags.Public);
        if (mi == null)
            continue;
        Console.WriteLine("====================================================");
        Console.WriteLine($"Running {asm.GetName().Name}");
        var passed = false;
        try
        {
            mi.Invoke(null, null);
            passed = true;
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
        }
        if (passed)
            Console.WriteLine($"Passed {asm.GetName().Name}");
    }
}

return 0;

Expected result:

$ MONO_ENV_OPTIONS=--interp DOTNET_MODIFIABLE_ASSEMBLIES=debug dotnet run -- /tmp/reproProps
>>> /tmp/reproProps/testProperty/testProperty.dll
====================================================
Running testProperty
 --- [testProperty]: Running test
Hello
 --- [testProperty]: Applying update 1
setting MyProp2 to hello2
 --- [testProperty]: Applying update 2
Applied second update
updated setting MyProp2 to hello2
setting MyProp3 to System.String[]
Passed testProperty

Actual result:

$ DOTNET_MODIFIABLE_ASSEMBLIES=debug dotnet run -- /tmp/reproProps
>>> /tmp/reproProps/testProperty/testProperty.dll
====================================================
Running testProperty
 --- [testProperty]: Running test
Hello
 --- [testProperty]: Applying update 1
setting MyProp2 to hello2
 --- [testProperty]: Applying update 2

Version info:

I'm running a "main" installer from https://github.com/dotnet/installer. 8.0.100-preview.5.23228.7

% dotnet --info
.NET SDK:
 Version:   8.0.100-preview.5.23228.7
 Commit:    0ce891843a

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.3
 OS Platform: Darwin
 RID:         osx.13-arm64
 Base Path:   /Users/alklig/work/net8-playground/dotnet/sdk/8.0.100-preview.5.23228.7/

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.0-preview.4.23225.14
  Architecture: arm64
  Commit:       9a7db5556f

.NET SDKs installed:
  8.0.100-preview.4.23225.3 [/Users/alklig/work/net8-playground/dotnet/sdk]
  8.0.100-preview.4.23228.11 [/Users/alklig/work/net8-playground/dotnet/sdk]
  8.0.100-preview.5.23228.7 [/Users/alklig/work/net8-playground/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.0-preview.4.23224.5 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-preview.4.23225.8 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-preview.5.23226.1 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23224.7 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23225.10 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23225.14 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/Users/alklig/work/net8-playground/dotnet]
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 1, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 1, 2023
@ghost
Copy link

ghost commented May 1, 2023

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Applying cumulative updates to properties in generic classes seems to shutdown coreclr.

In effect this is the code before the first update:

            using System;
            namespace testProperty;
            public class C1<T> {
                public T MyProp1 {get; set;}

                public static int M1() {
                    Console.WriteLine("Hello");
                    return 17;
                }
            }

Then the update changes it to:

                using System;
                namespace testProperty;
                public class C1<T> {
                    public T MyProp1 {get; set;}

                    public T MyProp2 {
                        get => default(T);
                        set {
                            Console.WriteLine($"setting MyProp2 to {value}");
                        }
                    }

                    public static int M1() {
                        var x = new C1<string>();
                        x.MyProp1 = "hello1";
                        x.MyProp2 = "hello2";
                        return 42;
                    }
                }

And finally change it to:

                using System;
                namespace testProperty;
                public class C1<T> {
                    public T MyProp1 {get; set;}

                    public T MyProp2 {
                        get => default(T);
                        set {
                            Console.WriteLine($"updated setting MyProp2 to {value}");
                        }
                    }
                    public T[] MyProp3 {
                        get => default(T[]);
                        set {
                            Console.WriteLine($"setting MyProp3 to {value}");
                        }
                    }

                    public static int M1() {
                        var x = new C1<string>();
                        x.MyProp1 = "hello1";
                        x.MyProp2 = "hello2";
                        x.MyProp3 = new string[] { "hello3" };
                        return 45;
                    }
                }

The actual invocations are done like this (ApplyUpdate is just loading the bytes from the .dmeta and .dil files and calling the System.Reflection.Metadata.MetadataUpdater.ApplyUpdate method):

        int x = testProperty.C1<int>.M1();
        ApplyUpdate();
        x = testProperty.C1<string>.M1();
        if (x != 42) {
            throw new Exception("Expected 42, got " + x);
        }
        PropertyInfo pInfo = typeof(testProperty.C1<string>).GetProperty("MyProp2");
        if (pInfo == null) {
            throw new Exception("Can't find newly added property: MyProp2");
        }
        ApplyUpdate(2);
        Console.WriteLine("Applied second update");
        x = testProperty.C1<string>.M1();
        if (x != 45) {
            throw new Exception("Expected 45, got " + x);
        }
        pInfo = typeof(testProperty.C1<string>).GetProperty("MyProp3");
        if (pInfo == null) {
            throw new Exception("Can't find newly added property: MyProp3");
        }
        MethodInfo mInfo = pInfo.GetGetMethod();
        if (mInfo == null) {
            throw new Exception("Can't find newly added property getter: MyProp3");
        }

I'm including a zip file with a .dll and .dmeta and .dil files with the changes. Use the following runner program to run it after unzipping the test case into some directory.

reproProps.zip

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection;

if (args.Length != 1)
{
    Console.WriteLine("Usage: generic-runner <directory>");
    return 1;
}

string directory = args[0];

if (!Directory.Exists(directory))
{
    Console.WriteLine("Directory does not exist: {0}", directory);
    return 1;
}


foreach (var subdir in Directory.GetDirectories(directory))
{
[reproProps.zip](https://github.com/dotnet/runtime/files/11367186/reproProps.zip)
    var alc = System.Runtime.Loader.AssemblyLoadContext.Default;

    var assemblies = Directory.GetFiles(subdir, "*.dll");
    foreach (var assembly in assemblies)
    {
        Console.WriteLine ($">>> {assembly}");
        var asm = alc.LoadFromAssemblyPath(assembly);
        var type = asm.GetType("HotReloadTest", throwOnError: false);
        if (type == null)
            continue;
        var mi = type.GetMethod("Run", BindingFlags.Static | BindingFlags.Public);
        if (mi == null)
            continue;
        Console.WriteLine("====================================================");
        Console.WriteLine($"Running {asm.GetName().Name}");
        var passed = false;
        try
        {
            mi.Invoke(null, null);
            passed = true;
        }
        catch (Exception e)
        {
            Console.WriteLine(e);
        }
        if (passed)
            Console.WriteLine($"Passed {asm.GetName().Name}");
    }
}

return 0;

Expected result:

$ MONO_ENV_OPTIONS=--interp DOTNET_MODIFIABLE_ASSEMBLIES=debug dotnet run -- /tmp/reproProps
>>> /tmp/reproProps/testProperty/testProperty.dll
====================================================
Running testProperty
 --- [testProperty]: Running test
Hello
 --- [testProperty]: Applying update 1
setting MyProp2 to hello2
 --- [testProperty]: Applying update 2
Applied second update
updated setting MyProp2 to hello2
setting MyProp3 to System.String[]
Passed testProperty

Actual result:

$ DOTNET_MODIFIABLE_ASSEMBLIES=debug dotnet run -- /tmp/reproProps
>>> /tmp/reproProps/testProperty/testProperty.dll
====================================================
Running testProperty
 --- [testProperty]: Running test
Hello
 --- [testProperty]: Applying update 1
setting MyProp2 to hello2
 --- [testProperty]: Applying update 2

Version info:

I'm running a "main" installer from https://github.com/dotnet/installer. 8.0.100-preview.5.23228.7

% dotnet --info
.NET SDK:
 Version:   8.0.100-preview.5.23228.7
 Commit:    0ce891843a

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.3
 OS Platform: Darwin
 RID:         osx.13-arm64
 Base Path:   /Users/alklig/work/net8-playground/dotnet/sdk/8.0.100-preview.5.23228.7/

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.0-preview.4.23225.14
  Architecture: arm64
  Commit:       9a7db5556f

.NET SDKs installed:
  8.0.100-preview.4.23225.3 [/Users/alklig/work/net8-playground/dotnet/sdk]
  8.0.100-preview.4.23228.11 [/Users/alklig/work/net8-playground/dotnet/sdk]
  8.0.100-preview.5.23228.7 [/Users/alklig/work/net8-playground/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.0-preview.4.23224.5 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-preview.4.23225.8 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-preview.5.23226.1 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23224.7 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23225.10 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-preview.4.23225.14 [/Users/alklig/work/net8-playground/dotnet/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  DOTNET_ROOT       [/Users/alklig/work/net8-playground/dotnet]
Author: lambdageek
Assignees: -
Labels:

area-Diagnostics-coreclr, untriaged, needs-area-label

Milestone: -

@lambdageek lambdageek removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 1, 2023
@lambdageek lambdageek added this to the 8.0.0 milestone May 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 1, 2023
@lambdageek
Copy link
Member Author

@lambdageek
Copy link
Member Author

FYI I am on osx-arm64 but this probably reproduces on other platforms, although I ahven't tried it

@lambdageek
Copy link
Member Author

Note that it is crashing before it prints out Applied second update. so my best guess is that coreclr doesn't like the second .dmeta file for some reason

@AaronRobinsonMSFT
Copy link
Member

We've no platform specific code in the new code paths so this should repro on any platform. @mikelle-rogers Could you take a look using a Checked runtime on Windows? If not, I can take a look tomorrow.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 2, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants