RFR (S) 8182397: Race in field updates when creating ArrayKlasses can lead to crash
erik.osterlund at oracle.com
Tue Jul 25 11:13:29 UTC 2017
On 2017-07-25 11:12, Andrew Haley wrote:
> On 25/07/17 09:29, Erik Österlund wrote:
>> On 2017-07-24 19:40, Andrew Haley wrote:
>>> On 24/07/17 17:44, Erik Osterlund wrote:
>>>> Sorry for jumping in to the conversation a bit late.
>>>> Since David Holmes is on vacation, I have to take his place in
>>>> questioning the elision of acquire on the reader side being
>>>> okay. Because I know he would if he was here.
>>>> In hotspot we almost *never* elide the acquire on the reader side,
>>>> relying on dependent loads.
>>> I'm sure we do in plenty of places, it's just undocumented or maybe
>>> even unknown to the original author.
>> That should be purged from the code. With fire.
> Possibly, but let us not burn ourselves in the process.
>>> We certainly do this a lot in the code that the JIT compilers
>>> generate, as you note below.
>> That is more okay when we are in control of the code generation.
>> Consume-like semantics have the following levels of support:
>> 1) No required support: Machines with strong enough memory ordering to
>> guarantee all normal loads and stores have acquire/release semantics
>> require no support.
>> 2) Compiler support: Some machines can rely on dependent loads, but what
>> looks like a dependent load in the C++ source code might not translate
>> to dependent loads in the compiled machine code. In order to enforce
>> that, the data dependency chains from the source code must be retained
>> in the generated machine code.
> Yes, yes, we know. :-)
>> As for the JIT-compiled code - it is fine to use consume-like semantics
>> if we control the code generation and know that the generated machine
>> code will respect the data dependencies that we wanted it to. But since
>> we do not control the C++ code generation, and do not have access to
>> tools like C++11 consume semantics, we can not just elide acquire and
>> hope for the best. Instead we have to use acquire until OrderAccess
>> starts supporting consume (if it ever will).
> OrderAccess could support consume tomorrow: that's up to us.
We could indeed. But then we got to do that first, and replace
load_acquire with load_consume, rather than with normal loads.
>> What we perhaps could do is to add shared code to OrderAccess for
>> load_consume, implement them as load_acquire, so that we at least
>> remember where in the code we could have gotten away with dependent
>> loads and exploit it, and later on plug it into C++11 consume so that
>> C++ compilers that support such contracts can utilize it on a platform
>> by platform basis.
> I suspect we could make it work immediately, because we know how to
> emit compiler fences on all platforms (do we?) and almost all of them
> respect data dependencies in hardware anyway. Those that don't will
> have to use acquire.
I think how we would implement consume warrants a new RFE and
discussion. I suspect this thread could become long off-topic discussion
otherwise. I am happy for now if we just agree that we got to have some
kind of consume implementation before we start removing acquire and
replacing it with that today non-existent load_consume.
Having said that...
I do not know if I agree compiler fences is not enough to ensure that
data dependencies in the generated machine code is preserved from the
For example, take this example pseudo code for performing what I refer
to as a stable load between two fields modified concurrently with
potential ABA issues:
x_start = load_relaxed(field_A)
y = load_consume(field_B)
x = load_consume(field_A)
if (x_start == x) break;
// use x->foo
In this example, the loop finishes when the loaded value from field_A
remains stable across a load of field_B. But it could have changed
in-between. But the choice of register for x used in x->foo after the
loop could be either the one corresponding to the x_start or x variables
in the pseudo code due to the arithmetic equality. One choice will
retain the intended data dependency to the load_consume(field_A), and
the other choice will not. If the choice of base register in x->foo is
equal to x_start (which seems perfectly valid), then the data dependency
is to a possibly stale load that survived an ABA check but might have
pointed to a different object that was concurrently recycled, whereas if
the base register is the same as x in the loop, then the data dependency
correctly corresponds to the load that had consume semantics, and is not
a stale value.
As a result, the generated machine code does not necessarily retain the
data dependencies from the source code if we only used compiler
barriers. I think we need C++11 consume specifically (or acquire) to be
>>> I don't think it's that much of a big deal, really. Putting in an
>>> acquire fence every time we read a java mirror strikes me as madness
>>> when consume is all we need on any target. Even Power PC doesn't need
>>> a fence instruction there. At least make sure that targets can turn
>>> that off: we should not pay the price for something we don't need.
>> Conversely, those targets would pay the price of compromised
>> correctness - a price higher than performance regressions.
>> Performance can only be optimized within the legal bounds of
> Please, spare us the lecture. No-one was sugesting compromising
Okay. It seemed to me like treating load_consume as just a normal load
would compromise correctness (due to lack of guarantees the dependency
chain is retained in generated code) and violate current hotspot
practices. And it seemed to me like this is precisely what was
suggested. I am sorry if I misunderstood something here...
>> Therefore we can only elide fences if we have well defined memory
>> ordering semantics for doing that and somehow control that the
>> generated code adheres to those semantics and respects them. We
>> simply do not have consume semantics today, and therefore to ensure
>> correctness, we must use acquire, until the day comes when we
>> support acquire.
> I guess you meant "consume" here? Anyway, all we need is a compiler
More information about the hotspot-dev