[aarch64-port-dev ] RFR(S): 8248657: Windows: strengthening in ThreadCritical regarding memory model

[Kim Barrett]

> On Jul 11, 2020, at 9:14 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Ludovic,
> 
> Sorry but this fix seems specific to the Windows-Aarch64 port work and as such should be fixed as part of that port when the JEP is approved and targeted.
> 
> David

I'm inclined to disagree with that assessment.

This change seems to me to make the code noticeably simpler and easier
to understand, while also making it platform-independent (eliminating
a non-TSO race). Those seem like good things, regardless of any
aarch64 port that might be coming.

My only (very small) quibble with it is that I think it could be made
simpler still by using a thread-safe function scoped static variable
to control the initialization, rather than using InitOnceExecuteOnce.
But unfortunately, that thread-safety guarantee is a C++11 feature. I
expect that feature is present in VS 2015 or later, but until JEP 347
gets integrated (very soon, I hope), we still support older versions.

So I think this change looks good.


> On 10/07/2020 8:19 am, Ludovic Henry wrote:
>> Hello,
>> This small fix is in the context of the larger support for Windows-AArch64. I am using Bernhard Urban's CR as I am currently not an author.
>> ThreadCritical is used to synchronize the allocation of new Arena chunks. However, on platforms with weaker memory models than x86 (primarily ARM), the original ThreadCritical initialization code would be racy, leading to crashes. To fix that, we switch to initializing the ThreadCritical static data by using a functionally-sound Win32 API focused on initialization [1]. This approach also has the advantage of simplifying the code, and get it closer to how it is done on Linux.
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8248657
>> Webrev: http://cr.openjdk.java.net/~burban/luhenry/8248657/webrev.00/
>> Testing: jtreg:test/hotspot/jtreg:tier1, jtreg:test/jdk:tier1, jtreg:test/jdk:tier2, jtreg:test/langtools on Windows-x86_64, no regressions
>> Thank you,
>> --
>> Ludovic
>> [1] https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-initonceinitialize