Skip to content

Conversation

@jakobbotsch
Copy link
Member

There is no guarantee that the JIT does not introduce a temporary for
the read of the fields obj1 and obj2 when they are passed to the
function calls, which might keep them alive even when nulling out the
fields. This happens when CFG is enabled, causing the test to fail.

Change the test to access the fields in separate methods marked as
noinline to fix the problem.

There is no guarantee that the JIT does not introduce a temporary for
the read of the fields `obj1` and `obj2` when they are passed to the
function calls, which might keep them alive even when nulling out the
fields. This happens when CFG is enabled, causing the test to fail.

Change the test to access the fields in separate methods marked as
noinline to fix the problem.
@ghost ghost assigned jakobbotsch Apr 12, 2022
@ghost ghost added the area-GC-coreclr label Apr 12, 2022
@ghost
Copy link

ghost commented Apr 12, 2022

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

Issue Details

There is no guarantee that the JIT does not introduce a temporary for
the read of the fields obj1 and obj2 when they are passed to the
function calls, which might keep them alive even when nulling out the
fields. This happens when CFG is enabled, causing the test to fail.

Change the test to access the fields in separate methods marked as
noinline to fix the problem.

Author: jakobbotsch
Assignees: jakobbotsch
Labels:

area-GC-coreclr

Milestone: -

Copy link
Member

@Maoni0 Maoni0 left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for making this fix!

@jakobbotsch jakobbotsch merged commit 5c57f2c into dotnet:main Apr 13, 2022
@jakobbotsch jakobbotsch deleted the fix-ReRegisterForFinalize-GC-test branch April 13, 2022 07:24
@ghost ghost locked as resolved and limited conversation to collaborators May 13, 2022
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.

2 participants