[wasm][coreclr] Fix methodesc reset for dynamic methods#122444
[wasm][coreclr] Fix methodesc reset for dynamic methods#122444radekdoulik merged 2 commits intodotnet:mainfrom
Conversation
Fix dotnet#122419 Make sure we don't enter the branch for rental methods in other cases when using portable entrypoints.
radekdoulik
requested review from
AaronRobinsonMSFT,
Copilot,
janvorli,
jkotas and
pavelsavara
|
Tagging subscribers to this area: @mangod9 |
There was a problem hiding this comment.
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()
|
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: |
jkotas
left a comment
jkotas
left a comment
There was a problem hiding this comment.
LGTM, assuming there are no regressions in the CI
|
The CI fails in LibraryImportGeneratorTests on Linux x64/arm64 both Azure Linux and Alpine with It fails on Windows too, but the assert is not logged into the console. |
|
It may be unrelated though |
|
Ah, yes, it is unrelated, it is #122447 |
Fix #122419
Make sure we don't enter the branch for rental methods in other cases when using portable entrypoints.