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

Found a race condition where the LOH flag on a segment is set too late. #54839

Conversation

PeterSolMS
Copy link
Contributor

This gives another thread the chance to allocate in a fresh LOH region that doesn't have the LOH flag set just yet and trip over an assert in Object::ValidateInner.

The fix is simply to set the flag in get_new_region before the region is put on the list for the LOH generation.

…e. This gives another thread the chance to allocate in a fresh LOH region that doesn't have the LOH flag set just yet and trip over an assert in Object::ValidateInner.

The fix is simply to set the flag in get_new_region before the region is put on the list for the LOH generation.
@ghost
Copy link

ghost commented Jun 28, 2021

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

Issue Details

This gives another thread the chance to allocate in a fresh LOH region that doesn't have the LOH flag set just yet and trip over an assert in Object::ValidateInner.

The fix is simply to set the flag in get_new_region before the region is put on the list for the LOH generation.

Author: PeterSolMS
Assignees: -
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!

@PeterSolMS PeterSolMS merged commit 969840e into dotnet:main Jun 29, 2021
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Jun 29, 2021
…e. This gives another thread the chance to allocate in a fresh LOH region that doesn't have the LOH flag set just yet and trip over an assert in Object::ValidateInner. (dotnet#54839)

The fix is simply to set the flag in get_new_region before the region is put on the list for the LOH generation.
lewing pushed a commit that referenced this pull request Jul 19, 2021
* [wasm] Run browser tests on helix/windows

* Build just wasm/browsertests on helix/windows

* Use $(ChromiumRevision) in windows links

* Fix conditions

* Set PATH differently

* Use backslash in PATH on windows

* Try different version of chromium

* Pass scenario and browser host to build

And set browser path on windows to be able to start chrome
from xharness

* Try to get more info from the helix workitems

* Fix dir separator, add broser path

* Create WasmBuildSupportDir

* Revert "Try to get more info from the helix workitems"

This reverts commit 8807434.

* Put the dir cmds back, fix mkdir call

* More debug info

* Bump xharness

* Bump xharness again

With darc this time

* StressLogAnalyzer didn't print the number of messages correctly if it exceeded the int range (2 billion). (#54832)

Fix is to just use 64 bit ints instead.

* Found a race condition where the LOH flag on a segment is set too late. This gives another thread the chance to allocate in a fresh LOH region that doesn't have the LOH flag set just yet and trip over an assert in Object::ValidateInner. (#54839)

The fix is simply to set the flag in get_new_region before the region is put on the list for the LOH generation.

* Try to show the chrome logs

* Use different path for chrome logs

* Use newer image with font for chrome

The chrome was crashing, because it didn't find any sans-serif font.

* Increase timeouts

* Disable tests which timeout

* Remove debug calls

* Put back normal scenario

* Do not set scenario in build args

* Add browser sample exclusion

* Restore the platform matrix

* Remove the wasm build test changes

That will be handled in #54451

* Remove duplicate exclusion

* Suggested property name change

* Fix last merge

* Simplify condition

We don't pass Scenario anymore

* Include chrome and chromedriver in the payload

Co-authored-by: Peter Sollich <petersol@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Jul 29, 2021
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