races on flat values

[John Rose]

On 10 Aug 2022, at 7:38, Dan Heidinga wrote:

> Thanks for writing this up, John.
>
> This matches my thinking overall.  Looking for confirmation on one
> point below related to synchronized blocks & non-atomic values.
>
> On Wed, Jul 13, 2022 at 11:32 PM John Rose <john.r.rose at oracle.com> 
> wrote:
>>
>> So, let’s talk about the Java Memory Model and consequent JVM 
>> support for flattenable types.

>> …

>> One thing to observe in passing is that when a method like C::next 
>> runs, it has a value this which is on the stack, not in heap. This 
>> means that there can be no races on the fields of this during the 
>> execution of any method. So as the body of any C method executes, the 
>> fields this.array and this.index cannot change. This is as one would 
>> expect from final fields. But it’s true even if the original copy 
>> of this is being concurrently trashed by racing writes. This means 
>> the JIT cannot treat race-prone heap containers as spilled copies of 
>> this, to be reloaded at leisure.
>
> Agreed.  A field (V.*F) must be "sticky" after having been read and so
> we can't re-read from the heap containers.

Yet another way of saying this is that the JMM Read event that picked up 
some field value from the heap has a single, unambiguous value (as a 
bundle of scalar values, of course).  Races come from consulting 
multiple JMM Reads, not from any possible variation in a single JMM 
Read.  The value of a single JMM Read event can and must be modeled (by 
the JIT) as a bit pattern to be stored in registers or in spill slots, 
not a heap location subject to any extra JMM Read events.  This is not 
new; the JIT never re-reads a field value that was already read by a 
`getfield` instruction, unless there is a second `getfield` instruction.

>> It has to pick up any and all fields that it might need just once per 
>> field needed in an inlined method call. It might kill dead stores, of 
>> course. If the class of this is atomic, it must use an atomic to pick 
>> up this.*F* (or the parts needed) at one time; otherwise it can pick 
>> up the needed parts of this.*F* as needed, but at most once per part.
>
> I think this is right but I'm still slightly uncomfortable with
> picking up the "needed parts of this.*F* as needed" as it can greatly
> expand the race window in user code in ways that aren't obvious from
> the source code.  Since this only applies to racy programs, the
> correct answer for the programmer - regardless of the size of the
> window - is to properly synchronize the access.

Yes.  I guess my point is that a race from two back-to-back reads (of 
adjacent locations, say) is not qualitatively different from a race from 
the same two reads separated by other stuff.  The two scenarios will 
show the same kinds of races, just at different rates (probabilities).

> In this model, when do the reads of fields actually occur for
> synchronized blocks?  In the following code, all C.val fields used
> after the block must be read during the synchronized block, correct?

Correct.  The JMM sees just one Read event that is trapped in the block 
by bracketing synchronization events.  It doesn’t matter (for this 
point) that the value of the Read consists of two scalars (array/index). 
  The value must be sampled between the block brackets.

Looking closer, the Read event decomposes to two scalar sub-Reads.  
Those might happen in separate hardware instructions, or a single 
non-atomic hardware instruction.  In such a case, a non-synchronized 
Write event in another thread, using a non-atomic sequence of 
instructions, could cause a race.  (Same as with `long` tearing in 
today’s Java.)

> static C.val sharedVal = ....;
>
> C.val myVal;
> synchronized(someLock) {
>    myVal = sharedVal;
> }
> ... myVal.array ...
> ... myVal.index
>
> Assuming all the subsequent field reads through myVal are guaranteed
> to have been privatized by the end of the synchronized block, then I
> think the model makes sense and sounds right to me.

Thanks for giving it a close look!

> @Tobi, have the rest of the OpenJ9 JIT developers weighed in on this 
> model yet?

>> …
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/valhalla-spec-observers/attachments/20220810/9cd2e1d1/attachment.htm>