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/6.0-rc1] [interp] Use existing InterpMethod if allocation and lookup race #57985

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Aug 23, 2021

Backport of #57968 to release/6.0-rc1

/cc @lambdageek

#57812

Customer Impact

In mobile devloop scenarios (which will use the interpreter by default), multi-threaded programs that execute the same method on multiple threads may crash.

Testing

CI

Risk

Low - on wasm we're single-threaded; on mobile in release configurations we don't use the interpreter. And as a mitigation, it is possible to use the JIT for debug configurations with both Android and iOS.

If two threads both want to get an InterpMethod for the same MonoMethod, and
they both see null from the first hash table lookup, make sure that whichever
one comes into the jit_mm lock second re-uses the previously inserted
InterpMethod, instead of its own version.

Without this change, racing threads will overwrite
MonoJitInfo:seq_points (in mono_interp_transform_method) which sometimes leads
to deallocating the same sequence points multiple times.

Fixes #57812
@ghost
Copy link

ghost commented Aug 23, 2021

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

Issue Details

Backport of #57968 to release/6.0-rc1

/cc @lambdageek

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-Codegen-Interpreter-mono

Milestone: -

@lambdageek lambdageek added the Servicing-consider Issue for next servicing release review label Aug 23, 2021
@lambdageek
Copy link
Member

@marek-safar ok for 6.0?

@akoeplinger akoeplinger added this to the 6.0.0 milestone Aug 24, 2021
@SteveMCarroll SteveMCarroll added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 24, 2021
@steveisok steveisok merged commit 949a0a0 into release/6.0-rc1 Aug 24, 2021
@steveisok steveisok deleted the backport/pr-57968-to-release/6.0-rc1 branch August 24, 2021 19:27
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2021
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.

Loader/classloader/regressions/523654/test532654_b/test532654_b.sh failing in CI
5 participants