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

[release/7.0] [mono][interp] Add unbox when calling valuetype method through delegate #79614

Merged

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Dec 13, 2022

If we are calling an open instance delegate, where the target method is on a valuetype, we will need to unbox this pointer.

Backport of #79445

Fixes #79354

Customer Impact

MAUI iOS applications making use of the popular AutoMapper NuGet library can crash in debug builds. Workaround is to disable mono interpreter and implicitly hot reload. Other platforms using interpreter, like blazor wasm, can potentially hit this issue even in Release.

Testing

Verified fix on sample app provided in the bug report and created a simple test case that is included in our suite.

Risk

Low. The fix only touches the code path for a very specific type of delegate invocation pattern which was previously handled incorrectly before.

If we are calling an open instance delegate, where the target method is on a valuetype, we will need to unbox this pointer.
@ghost
Copy link

ghost commented Dec 13, 2022

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

Issue Details

If we are calling an open instance delegate, where the target method is on a valuetype, we will need to unbox this pointer.

Backport of #79445

Fixes #79354

Customer Impact

MAUI iOS applications making use of the popular AutoMapper NuGet library can crash in debug builds. Workaround is to disable mono interpreter and implicitly hot reload. Other platforms using interpreter, like blazor wasm, can potentially hit this issue even in Release.

Testing

Verified fix on sample app provided in the bug report and created a simple test case that is included in our suite.

Risk

Low. The fix only touches the code path for a very specific type of delegate invocation pattern which was previously handled incorrectly before.

Author: BrzVlad
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@SamMonoRT SamMonoRT added the Servicing-consider Issue for next servicing release review label Dec 13, 2022
@SamMonoRT SamMonoRT added this to the 7.0.x milestone Dec 13, 2022
@BrzVlad
Copy link
Member Author

BrzVlad commented Dec 13, 2022

From the original PR, this doesn't contain the 846f4ea commit, since it is unnecessary risk for backport PR

@SamMonoRT
Copy link
Member

@carlossanlop - this is approved via Tactics email, but we should wait till @BrzVlad confirms the CI failures are not relevant.

@SamMonoRT SamMonoRT added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 13, 2022
@BrzVlad BrzVlad force-pushed the backport-interp-virt-delegate-vt branch from 7ced39b to dc8a8aa Compare December 14, 2022 07:50
@BrzVlad BrzVlad force-pushed the backport-interp-virt-delegate-vt branch from dc8a8aa to 61d4333 Compare December 14, 2022 11:08
@BrzVlad
Copy link
Member Author

BrzVlad commented Dec 14, 2022

I'm not seeing any failures with this

@carlossanlop carlossanlop modified the milestones: 7.0.x, 7.0.3 Jan 4, 2023
@carlossanlop
Copy link
Member

Approved by Tactics (7.0.3).
Signed off by area owner.
CI failure is known/unrelated/already fixed #78778 .
No OOB changes needed.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 77237c6 into dotnet:release/7.0 Jan 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants