Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

dpodder
Copy link

@dpodder dpodder commented Jul 8, 2017

Enabling PGO optimizations exposed a latent but in the ZeroMemoryInGCHeap function: it assumed that writing a constant (0) to a pointer-sized location in memory was inherently atomic. This is not, in fact, the case: the compiler can freely turn the write into a non-atomic sequence of smaller writes. This leads to a race condition with the GC running in another thread, as discovered by the test failure tracked by #12207.

The fix is essentially the same as for #4341, and corroborated by advice from compiler devs: mark the buffer being cleared as volatile. This prevents the compiler from performing read/write optimizations against the buffer, and thus, that the store of a pointer-sized integer constant (given that the buffer access is aligned) remains a single, atomic store.

by adding the volatile keyword (which disables memset optimization).
Fixes #12207
@dpodder
Copy link
Author

dpodder commented Jul 8, 2017

@swgillespie @Petermarcu @brianrob @cmckinsey @Maoni0 - This should fix #12207 in master. Assuming it's green, we will want to port it to release/2.0.0 ASAP to get the the blocking bug cleared for 2.0.

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.

Leave a comment

@dpodder
Copy link
Author

dpodder commented Jul 8, 2017

@dotnet-bot test windows_nt release longgc

jkotas added a commit to dotnet/corert that referenced this pull request Jul 8, 2017
@jkotas jkotas merged commit c28b3c9 into dotnet:master Jul 8, 2017
@dpodder dpodder deleted the fix-12207 branch July 8, 2017 07:56
@karelz karelz modified the milestone: 2.1.0 Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants