Skip to content

Conversation

@radekdoulik
Copy link
Member

Fix #122419

Make sure we don't enter the branch for rental methods in other cases when using portable entrypoints.

Fix dotnet#122419

Make sure we don't enter the branch for rental methods in other cases
when using portable entrypoints.
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug in the MethodDesc::Reset() function that affected WebAssembly and CoreCLR when using portable entrypoints. The fix ensures that the "rental methods" code path (for reflection.emit dynamic methods) is only entered when appropriate, preventing non-rental methods from incorrectly executing that branch.

Key Changes:

  • Modified conditional compilation logic to add an explicit check for reflection emit modules when portable entrypoints are enabled
  • Changed from unconditional execution of the rental methods block to conditional execution based on IsReflectionEmit()

@jkotas
Copy link
Member

jkotas commented Dec 11, 2025

Rental methods were .NET Framework feature. They do not exist in .NET Core. The support for the rental methods should be unreachable code.

I think the right fix is to change this to just:

#ifndef FEATURE_PORTABLE_ENTRYPOINTS
   _ASSERTE(HasPrecode());
   GetPrecode()->Reset();
#endif // !FEATURE_PORTABLE_ENTRYPOINTS

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming there are no regressions in the CI

@janvorli
Copy link
Member

The CI fails in LibraryImportGeneratorTests on Linux x64/arm64 both Azure Linux and Alpine with
/__w/1/s/src/coreclr/interop/comwrappers.cpp:519: ULONG ManagedObjectWrapper::Release(): Assertion `(!"Over release of MOW - COM")' failed.

It fails on Windows too, but the assert is not logged into the console.

@janvorli
Copy link
Member

It may be unrelated though

@janvorli
Copy link
Member

Ah, yes, it is unrelated, it is #122447

@radekdoulik radekdoulik merged commit 487ff1b into dotnet:main Dec 11, 2025
93 of 99 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[browser][coreCLR] Assert GetLoaderModule()->IsReflectionEmit() for InvokeStub_AttributeUsageAttribute.set_AllowMultiple

3 participants