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.

Sign up for GitHub

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

Jump to bottom

Regression in .NET 8.0-preview.5: dependent assembly is not included in TPA list when running fxdependent pwsh build with preview.5 dotnet.exe #86713

Closed
daxian-dbw opened this issue May 24, 2023 · 26 comments
Closed

Regression in .NET 8.0-preview.5: dependent assembly is not included in TPA list when running fxdependent pwsh build with preview.5 dotnet.exe #86713

daxian-dbw opened this issue May 24, 2023 · 26 comments
Labels
area-AssemblyLoader-coreclr

Comments

@daxian-dbw
Copy link
Contributor

Description

This is root cause of the PowerShell issue Microsoft.Management.Infrastructure can't be loaded in latest .NET 8 Preview 5 build.

My investigation shows it’s a regression in .NET 8-preview.5.

The Microsoft.Management.Infrastructure.dll (MMI.dll) can be loaded in preview.5 without a problem by Assembly.LoadFrom. However, when running a fxdependent build of pwsh with the .NET 8.0-preview.5 SDK (through ....\dotnet.exe ....\pwsh.dll), the MMI.dll doesn’t get included in the Trusted_Platform_Assemblies (TPA) list, and thus it cannot be found when the loading is requested. As a comparison, when running the same fxdependent build of pwsh with the .NET 8-preview.4 SDK, the MMI.dll is included in the TPA list and hence the loading works as expected at pwsh startup.

The startup tracing logs for using the preview.5 (p5-trace.txt) and preview.4 (p4-trace.txt) SDKs are attached. The traces were generated by setting COREHOST_TRACE = 1. The build produced pwsh.deps.json file for the fxdependent build is also attached for your reference.

I want to call out that the Microsoft.Management.Infrastructure NuGet package ships netstandard1.6 managed assemblies (MMI.dll). Maybe the regression in .NET 8 preview.5 is related to the netstandard1.6 TFM? Just a guess.

p5-trace.txt
p4-trace.txt
pwsh.deps.json.txt

The .NET 8.0-preview.5 build I'm using is 8.0.100-preview.5.23269.4.

Reproduction Steps

See the repro steps in PowerShell/PowerShell#19679.
Also, the tracing logs should provide lots of information about the issue.

Expected behavior

The fxdependent pwsh build works as expected when using with the .NET 8.0-preview.5 build.

Actual behavior

Crash at startup because the MMI.dll cannot be found -- it's not included in the TPA list.

Regression?

Yes. it's a regression from .NET 8.0-preview.4.

Known Workarounds

N/A

Configuration

PS:13> ..\dotnet8-p5\dotnet.exe --version
8.0.100-preview.5.23269.4

Win10-x64 platform

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 24, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 24, 2023
@daxian-dbw daxian-dbw mentioned this issue May 24, 2023
Closed
5 tasks
@ghost
Copy link

ghost commented May 24, 2023

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

Issue Details

Description

This is root cause of the PowerShell issue Microsoft.Management.Infrastructure can't be loaded in latest .NET 8 Preview 5 build.

My investigation shows it’s a regression in .NET 8-preview.5.

The Microsoft.Management.Infrastructure.dll (MMI.dll) can be loaded in preview.5 without a problem by Assembly.LoadFrom. However, when running a fxdependent build of pwsh with the .NET 8.0-preview.5 SDK (through ....\dotnet.exe ....\pwsh.dll), the MMI.dll doesn’t get included in the Trusted_Platform_Assemblies (TPA) list, and thus it cannot be found when the loading is requested. As a comparison, when running the same fxdependent build of pwsh with the .NET 8-preview.4 SDK, the MMI.dll is included in the TPA list and hence the loading works as expected at pwsh startup.

The startup tracing logs for using the preview.5 (p5-trace.txt) and preview.4 (p4-trace.txt) SDKs are attached. The traces were generated by setting COREHOST_TRACE = 1. The build produced pwsh.deps.json file for the fxdependent build is also attached for your reference.

I want to call out that the Microsoft.Management.Infrastructure NuGet package ships netstandard1.6 managed assemblies (MMI.dll). Maybe the regression in .NET 8 preview.5 is related to the netstandard1.6 TFM? Just a guess.

p5-trace.txt
p4-trace.txt
pwsh.deps.json.txt

The .NET 8.0-preview.5 build I'm using is 8.0.100-preview.5.23269.4.

Reproduction Steps

See the repro steps in PowerShell/PowerShell#19679.
Also, the tracing logs should provide lots of information about the issue.

Expected behavior

The fxdependent pwsh build works as expected when using with the .NET 8.0-preview.5 build.

Actual behavior

Crash at startup because the MMI.dll cannot be found -- it's not included in the TPA list.

Regression?

Yes. it's a regression from .NET 8.0-preview.4.

Known Workarounds

N/A

Configuration

PS:13> ..\dotnet8-p5\dotnet.exe --version
8.0.100-preview.5.23269.4

Win10-x64 platform

Other information

No response

Author: daxian-dbw
Assignees: -
Labels:

area-AssemblyLoader-coreclr, untriaged, needs-area-label
Milestone: -

@mthalman
Copy link
Member

@agocke - This is Preview 5 related and blocking our ability to ship PowerShell in the official Docker images for that release.

@agocke
Copy link
Member

agocke commented May 24, 2023

@elinor-fung Could you take a look? Maybe related to the RID changes?

@vitek-karas
Copy link
Member

It's very likely the RID changes (the package is 3 years old on NuGet, but it used on-portable RIDs back then).

@daxian-dbw could you please try to modify the powershell runtimeconfig.json with the workaround mentioned in here: dotnet/docs#35398

Specifically add a runtime property System.Runtime.Loader.UseRidGraph: true.

@github-project-automation github-project-automation bot moved this to Backlog in .NET Docker May 24, 2023
@mthalman mthalman moved this from Backlog to Tracking in .NET Docker May 24, 2023
@daxian-dbw
Copy link
Contributor Author

@vitek-karas I tried it, but it results in the same crash.

image

@elinor-fung
Copy link
Member

Can you try "System.Host.Resolution.ReadRidGraph": true? We recently renamed the setting to System.Runtime.Loader.UseRidGraph, so the build you are on might not have the new name.

@daxian-dbw
Copy link
Contributor Author

@elinor-fung Sure. I tried it again and this time it works, the crash is gone after adding "System.Host.Resolution.ReadRidGraph": true to pwsh.runtimeconfig.json.

@elinor-fung
Copy link
Member

elinor-fung commented May 25, 2023 •
edited
Loading

Thanks @daxian-dbw. That is the fallback/compat switch we have for packages that currently use non-portable RIDs. The recommendation for packages is to more to portable RIDs - and if any assets are truly version/distro specific, handle custom loading logic through existing .NET APIs. We can work with powershell to transition Microsoft.Management.Infrastructure.Runtime.Win - are you the right person (and if not, could you point me at them)?

@mthalman
Copy link
Member

It looks like @elinor-fung has offered a long-term plan to resolve this.

But in the short term, we need to figure out what to do so we can ship the Preview 5 Docker images.

@elinor-fung, is the use of this config property something that should be acceptable for shipping PowerShell in this configuration to customers in this Preview release?

@daxian-dbw, are we able to get a new release of PowerShell in time for the .NET 8 Preview 5 release (June 13)? If not, would it be acceptable for us to modify the C:\Program Files\powershell\.store\powershell.windows.x64\7.4.0-preview.3\powershell.windows.x64\7.4.0-preview.3\tools\net8.0\any\pwsh.runtimeconfig.json file as part of our installation logic in our Dockerfile so that it includes this property?

@elinor-fung
Copy link
Member

Yes, we should be able to use the config to unblock shipping the Docker images.

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 25, 2023
@daxian-dbw
Copy link
Contributor Author

daxian-dbw commented May 25, 2023 •
edited
Loading

@mthalman I don't think we will be able to have a new release before June 13. Feel free to make the necessary changes in your docker build to unblock the .NET 8 preview.5 release.

@elinor-fung We own the Microsoft.Management.Infrastructure package but we don't own the corresponding code. It's legacy managed C++ code that only builds in Windows build. Loop in @adityapatwardhan who I believe did the packaging work for the 2.0 version.

Can you please share some pointers for us to better understand what "portable" and "non-portable" RID means?
Also, if this is an intentional breaking change, is there a way to opt-out it in the build (I'm aware that runtimeconfig.template.json can be used during build to pre-define runtime configuration options, but will there be a build property that can be used in .csproj file to disable this feature?)

@daxian-dbw
Copy link
Contributor Author

Just want to call out on the impact -- applications that host PowerShell 7+ (i.e. using the 7.2+ version PowerShell NuGet packages) will run into the same crash when they update to .NET 8 due to this breaking change. PowerShell Azure Function is an example of those applications.

@elinor-fung
Copy link
Member

The RuntimeHostConfigurationOption is the build project equivalent of using that template. For example:

<RuntimeHostConfigurationOption Include="System.Runtime.Loader.UseRidGraph" Value="true" />

By 'portable' RIDs, we mean the non-version-specific, non-distribution-specific RIDs (sorry, 'portable' is rather overloaded). Those RIDs are the ones we are referring to in the RID doc with:

RIDs that aren't tied to a specific version or OS distribution are the preferred choice

In that same doc's list of examples for different systems, the 'portable' RIDs are the ones described as 'not version-specific' or 'not distribution-specific'.

'Non-portable' refers to all the other RIDs that are version-specific and/or distribution-specific.

@agocke
Copy link
Member

agocke commented May 25, 2023 •
edited
Loading

So in this case, would adding a win-x64 (portable Windows) section be good enough?

@elinor-fung
Copy link
Member

Yeah, if those same assets were "rid": "win-x64" (instead of "rid": "win10-x64"), we would use them. From the package authoring side, that would be putting them in a runtimes/win-x64 subfolder.

@adityapatwardhan
Copy link

Unfortunately, the native DLLs in each of the RIDs are specific to the Windows OS version.

runtimes/win10-x64/native/mi.dll depends on the following:

  Dump of file C:\Users\adityap\Downloads\microsoft.management.infrastructure.runtime.win.2.0.0\runtimes\win10-x64\native\mi.dll
  
  File Type: DLL
  
    Image has the following dependencies:
  
      msvcrt.dll
      api-ms-win-core-heap-l1-1-0.dll
      api-ms-win-core-processthreads-l1-1-0.dll
      api-ms-win-security-base-l1-1-0.dll
      api-ms-win-core-errorhandling-l1-1-0.dll
      api-ms-win-core-handle-l1-1-0.dll
      api-ms-win-core-interlocked-l1-1-0.dll
      api-ms-win-core-libraryloader-l1-2-0.dll
      api-ms-win-core-string-l1-1-0.dll
      api-ms-win-core-localization-l1-2-0.dll
      api-ms-win-core-heap-l2-1-0.dll
      api-ms-win-core-synch-l1-2-0.dll
      api-ms-win-core-profile-l1-1-0.dll
      api-ms-win-core-sysinfo-l1-1-0.dll
      api-ms-win-core-rtlsupport-l1-1-0.dll
      ntdll.dll
      miutils.dll

runtimes/win7-x64/native/mi.dll - depends on the following:

Dump of file C:\Users\adityap\Downloads\microsoft.management.infrastructure.runtime.win.2.0.0\runtimes\win7-x64\native\mi.dll

File Type: DLL

  Image has the following dependencies:

    msvcrt.dll
    KERNEL32.dll
    ADVAPI32.dll
    USER32.dll
    ntdll.dll
    miutils.dll

@agocke
Copy link
Member

agocke commented May 25, 2023 •
edited
Loading

win7-x64 is the same as win-x64 for .NET 8, so at minimum you could just rename win7 to win. Note: this would mean that the win10 users would use the win7 binaries, but since the win7 dependencies are a subset of the win10 dependencies, that should be OK.

@agocke
Copy link
Member

agocke commented Jun 21, 2023

Actually, win7 and win81 are no longer supported in .NET 8 (dotnet/core#7556) -- you can just move the win10 DLL to the win-x64 TFM folder and everything should work.

@jaredpar jaredpar mentioned this issue Jun 22, 2023
Merged
@jaredpar
Copy link
Member

Note: this bug is blocking razor's ability to adopt .NET 8 Preview 5 as we use pwsh in our build pipelines.

@agocke
Copy link
Member

agocke commented Jun 22, 2023 •
edited
Loading

@daxian-dbw Where you able to make the changes I suggested? Note that this is an intentional breaking change, specified in https://learn.microsoft.com/en-us/dotnet/core/compatibility/deployment/8.0/rid-asset-list.

@daxian-dbw
Copy link
Contributor Author

We cannot make the changes because 1) we don't own the sources and can no longer build those assemblies 2) we cannot use the existing assemblies in that package to repackage due to compliance issue.

In fact, that package itself has compliance issue and our team is currently looking into feasibility to remove dependency on it. @adityapatwardhan and @SeeminglyScience may have more details to share.

@agocke
Copy link
Member

agocke commented Jun 23, 2023

You shouldn't need to rebuild any assemblies -- my suggestion is just to move the win10-x64 assembly to the win-x64 folder. It will be functionally identical to the current layout for .NET 8+. For .NET 6 you can use the win7-x64 assembly.

@daxian-dbw
Copy link
Contributor Author

daxian-dbw commented Jun 23, 2023 •
edited
Loading

  1. we cannot use the existing assemblies in that package to repackage due to compliance issue.

We cannot repackage the existing assemblies without a build pipeline. It violates the compliance as far as I know.

@mthalman
Copy link
Member

@daxian-dbw - At a minimum, you should be able to make the runtimeconfig change with the UseRidGraph setting, right?

@daxian-dbw
Copy link
Contributor Author

We can, for the PowerShell packages. @adityapatwardhan is working on the next preview release, so he can share more about the plan.

However, for any application out there that host PowerShell by using the PowerShell NuGet packages (e.g. System.Management.Automation), they will need to explicitly add this runtime config too, otherwise, they will run into the same problem.

@agocke agocke added this to AppModel Jul 10, 2023
@agocke
Copy link
Member

agocke commented Jul 24, 2023

Closing as this is not a bug in runtime -- the behavior is expected in .NET 8.

@agocke agocke closed this as completed Jul 24, 2023
@github-project-automation github-project-automation bot moved this from Tracking to Done in .NET Docker Jul 24, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-AssemblyLoader-coreclr
Projects
.NET Docker
Status: Done
AppModel
Archived in project
Milestone
No milestone
Development

No branches or pull requests
8 participants
@daxian-dbw @jaredpar @vcsjones @agocke @vitek-karas @adityapatwardhan @mthalman @elinor-fung