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

App throws TypeLoadException when published trimmed but works fine when published native AOT (no trim warnings) #102796

Closed
DamianEdwards opened this issue May 29, 2024 · 9 comments · Fixed by #102857
Assignees
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented May 29, 2024

Description

Repo: https://github.com/DamianEdwards/RazorSlices/tree/aot/samples/RazorSlices.Samples.WebApp

I'm updating my Razor Slices library to support native AOT and am running into a scenario where the sample app produces no warnings when published as trimmed or as native AOT, and runs successfully when published native AOT, but throws a TypeLoadException when running after publishing trimmed.

The issue appears to be an interface implementation being trimmed from a type that's generated by a source generator. The method in question is a static abstract interface member. Notably, this method is not called via the interface in a generic fashion anywhere, which differs from other generated types in this app using the same pattern. I'm assuming this is the cause of the issue.

image

image
 System.TypeLoadException: Type 'RazorSlices.Samples.WebApp.Slices._Layout' from assembly 'RazorSlices.Samples.WebApp, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' tried to override method 'Create' but does not implement or inherit that method.
         at AspNetCoreGeneratedDocument.Slices_Todos.GetLayout()
         at RazorSlices.RazorSlice.RenderToBufferWriterAsync(IBufferWriter`1 bufferWriter, Func`2 flushAsync, HtmlEncoder htmlEncoder, CancellationToken cancellationToken) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.cs:line 118
         at RazorSlices.RazorSlice.RenderAsync(IBufferWriter`1 bufferWriter, Func`2 flushAsync, HtmlEncoder htmlEncoder, CancellationToken cancellationToken) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.cs:line 87
         at RazorSlices.RazorSlicePipeWriterExtensions.RenderToPipeWriterAsync(RazorSlice, PipeWriter, HtmlEncoder , CancellationToken ) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.PipeWriterExtensions.cs:line 21
         at RazorSliceHttpResultHelpers.ExecuteAsync(RazorSlice, HttpContext, HtmlEncoder , Int32, String) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSliceHttpResultHelpers.cs:line 22
         at Microsoft.AspNetCore.Http.HttpResults.RazorSliceHttpResult`1.Microsoft.AspNetCore.Http.IResult.ExecuteAsync(HttpContext httpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSliceHttpResultOfTModel.cs:line 34
         at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F69328E0708B4B584C5AACA22FE2C51A1CF192D6622828F613FC57C583CA77B63__GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(IResult, HttpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\samples\RazorSlices.Samples.WebApp\Microsoft.AspNetCore.Http.RequestDelegateGenerator\Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator\GeneratedRouteBuilderExtensions.g.cs:line 1075
         at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F69328E0708B4B584C5AACA22FE2C51A1CF192D6622828F613FC57C583CA77B63__GeneratedRouteBuilderExtensionsCore.<>c__DisplayClass3_0.<MapGet1>g__RequestHandler|4(HttpContext httpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\samples\RazorSlices.Samples.WebApp\Microsoft.AspNetCore.Http.RequestDelegateGenerator\Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator\GeneratedRouteBuilderExtensions.g.cs:line 203
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
         at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1)

Note that if the app is also configured to publish self-contained, the error message does not include a description which makes it much harder to diagnose the root cause:

Connection id "0HN3VJ6M4UE06", Request id "0HN3VJ6M4UE06:00000001": An unhandled exception was thrown by the application.
      System.TypeLoadException
         at AspNetCoreGeneratedDocument.Slices_Todos.GetLayout()
         at RazorSlices.RazorSlice.RenderToBufferWriterAsync(IBufferWriter`1 bufferWriter, Func`2 flushAsync, HtmlEncoder htmlEncoder, CancellationToken cancellationToken) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.cs:line 118
         at RazorSlices.RazorSlice.RenderAsync(IBufferWriter`1 bufferWriter, Func`2 flushAsync, HtmlEncoder htmlEncoder, CancellationToken cancellationToken) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.cs:line 87
         at RazorSlices.RazorSlicePipeWriterExtensions.RenderToPipeWriterAsync(RazorSlice, PipeWriter, HtmlEncoder , CancellationToken ) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSlice.PipeWriterExtensions.cs:line 21
         at RazorSliceHttpResultHelpers.ExecuteAsync(RazorSlice, HttpContext, HtmlEncoder , Int32, String) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSliceHttpResultHelpers.cs:line 22
         at Microsoft.AspNetCore.Http.HttpResults.RazorSliceHttpResult`1.Microsoft.AspNetCore.Http.IResult.ExecuteAsync(HttpContext httpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\src\RazorSlices\RazorSliceHttpResultOfTModel.cs:line 34
         at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>FCB64269D8A4C26C2E072E32ACF06A96ADD7E2DEE99BD0C9B78931C09283D15A4__GeneratedRouteBuilderExtensionsCore.ExecuteAsyncExplicit(IResult, HttpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\samples\RazorSlices.Samples.WebApp\obj\Release\net8.0\win-x64\generated\Microsoft.AspNetCore.Http.RequestDelegateGenerator\Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator\GeneratedRouteBuilderExtensions.g.cs:line 1075
         at Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>FCB64269D8A4C26C2E072E32ACF06A96ADD7E2DEE99BD0C9B78931C09283D15A4__GeneratedRouteBuilderExtensionsCore.<>c__DisplayClass3_0.<MapGet1>g__RequestHandler|4(HttpContext httpContext) in D:\src\GitHub\DamianEdwards\RazorSlices\samples\RazorSlices.Samples.WebApp\obj\Release\net8.0\win-x64\generated\Microsoft.AspNetCore.Http.RequestDelegateGenerator\Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator\GeneratedRouteBuilderExtensions.g.cs:line 203
         at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
         at Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1)

Reproduction Steps

  1. Clone https://github.com/DamianEdwards/RazorSlices and checkout the aot branch
  2. Navigate to .\samples\RazorSlices.Samples.WebApp\
  3. Run dotnet publish to publish the app as native AOT
  4. Run .\bin\Release\net8.0\win-x64\publish\RazorSlices.Samples.WebApp.exe to run the published app
  5. Open a browser and navigate to http://localhost:5000/ and click around the pages of the site, seeing it works
  6. Stop the app
  7. Edit the RazorSlices.Samples.WebApp.csproj file so that <PublishAot>true</PublishAot> is commented out and <PublishTrimmed>true</PublishTrimmed>, e.g.:
    <!--<PublishAot>true</PublishAot>-->
    <PublishTrimmed>true</PublishTrimmed>
    <!--<PublishSingleFile>true</PublishSingleFile>-->

8.Run dotnet publish to publish the app trimmed
9. Run .\bin\Release\net8.0\win-x64\publish\RazorSlices.Samples.WebApp.exe to run the published app
10. Open a browser and navigate to http://localhost:5000/
11. Note the exception message in the terminal the app was run from

Expected behavior

As not trim warnings are produced on publish, and the app works as expected when published native AOT, I expect it to work when published trimmed.

Actual behavior

The app throws an exception at runtime after being published trimmed.

Regression?

Not sure but it repros with both SDK versions 8.0.106 and 8.0.300 on Windows x64

Known Workarounds

None

Configuration

Windows 11 x64
.NET SDK 8.0.106 and 8.0.300

Other information

No response

@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 29, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label May 29, 2024
@agocke agocke added this to the 9.0.0 milestone May 29, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label May 29, 2024
@MichalStrehovsky MichalStrehovsky added area-Tools-ILLink .NET linker development as well as trimming analyzers and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 29, 2024
Copy link
Contributor

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

@DamianEdwards
Copy link
Member Author

DamianEdwards commented May 29, 2024

Here's a minimal repro console app. Publish this native AOT it works, publish it trimmed it throws TypeLoadException. Repros on .NET 7 too:

// Get message via generic method
var message = MessageFactory.GetMessage<TypeWithStaticMessage>();
Console.WriteLine(message);

// Get message directly from type
var message2 = TypeWithStaticMessage2.GetMessage();
Console.WriteLine(message2);

public static class MessageFactory
{
    public static string GetMessage<T>() where T : ITypeWithStaticMessage
    {
        return T.GetMessage();
    }
}

public class TypeWithStaticMessage : ITypeWithStaticMessage
{
    // Force the string to have dynamic element so that it doesn't get compiled in as a literal
    public static string GetMessage() => $"Hello from {nameof(TypeWithStaticMessage)}, the time is {TimeOnly.FromDateTime(DateTime.Now)}";
}

public class TypeWithStaticMessage2 : ITypeWithStaticMessage
{
    // Force the string to have dynamic element so that it doesn't get compiled in as a literal
    public static string GetMessage() => $"Hello from {nameof(TypeWithStaticMessage2)}, the time is {TimeOnly.FromDateTime(DateTime.Now)}";
}

public interface ITypeWithStaticMessage
{
    abstract static string GetMessage();
}

@DamianEdwards
Copy link
Member Author

Figured out a workaround by changing the method name and then make the generated types explicitly implement the interface methods and forcing the non-interface methods to call through a generated factory method that then invokes the explicitly implemented methods via the interface. Fair amount of indirection but doing it this way results in the interface implementation on the types themselves not being stripped.

DamianEdwards added a commit to DamianEdwards/RazorSlices that referenced this issue Jun 1, 2024
DamianEdwards added a commit to DamianEdwards/RazorSlices that referenced this issue Jun 7, 2024
* hacky hacky hack

* More hacky

* WIP

* Update TODO comment

* More WIP

* WIP

* Supports full trimming now

* Update RazorSlices.Samples.WebApp.csproj

* Cleanup

* More cleanup

* Trying to add publish tests

* urgh round and round we go

* wip

* Working again

* Tweaks

* WIP

* Update RazorSliceFactory.cs

* Fixes for trimmed/non-AOT mode

* Add JSON config for AOT

* Remove unused code

* Remove more unused code

* Update README.md

* Source Generator (#41)

* Update README.md

* Add code generator

* Add type name validation

---------

Co-authored-by: Damian Edwards <damian@damianedwards.com>

* More generator cleanup

* More tweaks

* More tweaks

* Comment tweaks

* Remove props & make PathUtils honor OS

* Update CSharpCodeValidationTests.cs

* Update global.json

* Remove net6

* Fix RDG failing

* More links in footer

* Fix infinite loop in HTML encoding

Fixes #37

* Remove unused compiler directives

* Layouts working for BufferWriterCase

* Update RazorSlice.cs

* Add support for layout sections

* Ensure slices are disposed when using layouts

* Update _build.yml

* More fixes for #37

* Add some tests

* Update .gitignore

* Fix method names on RazorLayoutSlice

* WIP use interfaces for declaring use of layouts

* Tweaks

* Add another encoding test

* Update PublishSample.cs

* Remove unused code & add comments

* More comments

* Rename IUseLayout to IUsesLayout. More doc comments & things.

* Workaround for dotnet/runtime#102796

More comments too

* Ensure response has started before rendering slices

* Use PipeWriter directly instead of IBufferWriter<byte>

* Suppress warning

* Implement auto flushing with a pooled tracking PipeWriter

* Make initialize internal & other small tweaks

* Move RazorSlice writing methods into separate file

* Added benchmarks

* Separate benchmarks into separate apps

* Use default HtmlEncoder

* Refactor benchmarks

* Remove blank line

* Add benchmark using RazorSlices from nuget

* Add pride & prejudice benchmark

* Try to use build configurations for benchmarks

* Revert "Try to use build configurations for benchmarks"

This reverts commit b83495e.

* Try to use build configuration for benchmarks

* Benchmarks working

* More benchmarks updates

* More benchmarks

* Added lorem25 impl for RazorComponents

* Delete unused image files

* Move common benchmark props to Directory.Build.props

* Add a WriteHtml overload for writing a string value without HTML encoding

* Add more benchmarks

* Add BlazorSSR lorem to benchmarks

* Fix sln build configuration

* Add a micro benchmark comparing prev to local

FIxed stack overflow when rendering with layout to TextWriter too

* Add profiling harness console app

* Update README.md

---------

Co-authored-by: Oga <m26416083@alumni.petra.ac.id>
@DamianEdwards
Copy link
Member Author

Is this a change we'd backport to 8.x?

@jtschuster
Copy link
Member

I think it might be worth it to backport since it's an LTS. It's not a particularly convoluted corner case, the workaround isn't great, and the fix wasn't too big. @sbomer @agocke thoughts?

@agocke
Copy link
Member

agocke commented Jun 11, 2024

Seems reasonable , if we believe risk is low

@jtschuster
Copy link
Member

Figured out a workaround by changing the method name and then make the generated types explicitly implement the interface methods and forcing the non-interface methods to call through a generated factory method that then invokes the explicitly implemented methods via the interface.

@DamianEdwards Does the app work if you just added an explicit implementation that forwards to the public static method? I don't think the call through a factory method should be necessary. I haven't looked into all the usage patterns that would affect trimmed output, but the minimal repro works with this.

// Get message via generic method
var message = MessageFactory.GetMessage<TypeWithStaticMessage>();
Console.WriteLine(message);

// Get message directly from type
var message2 = TypeWithStaticMessage2.GetMessage();
Console.WriteLine(message2);

public static class MessageFactory
{
    public static string GetMessage<T>() where T : ITypeWithStaticMessage
    {
        return T.GetMessage();
    }
}

public class TypeWithStaticMessage : ITypeWithStaticMessage
{
+   static string ITypeWithStaticMessage.GetMessage() => GetMessage();
    // Force the string to have dynamic element so that it doesn't get compiled in as a literal
    public static string GetMessage() => $"Hello from {nameof(TypeWithStaticMessage)}, the time is {TimeOnly.FromDateTime(DateTime.Now)}";
}

public class TypeWithStaticMessage2 : ITypeWithStaticMessage
{
+   static string ITypeWithStaticMessage.GetMessage() => GetMessage();
    // Force the string to have dynamic element so that it doesn't get compiled in as a literal
    public static string GetMessage() => $"Hello from {nameof(TypeWithStaticMessage2)}, the time is {TimeOnly.FromDateTime(DateTime.Now)}";
}

public interface ITypeWithStaticMessage
{
    abstract static string GetMessage();
}

@DamianEdwards
Copy link
Member Author

Ah interesting. Let me update my generator to just do that and I'll let you know.

@DamianEdwards
Copy link
Member Author

DamianEdwards commented Jun 13, 2024

Yep that worked, thanks!

FYI DamianEdwards/RazorSlices#52

DamianEdwards added a commit to DamianEdwards/RazorSlices that referenced this issue Jun 13, 2024
DamianEdwards added a commit to DamianEdwards/RazorSlices that referenced this issue Jun 13, 2024
* Simplified for the workaround for dotnet/runtime#102796

* Update RazorSlices.Samples.WebApp.csproj
@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants