[java] org.antlr.v4.runtime.misc.Interval.of is not thread safe

[lukasz-stec]

The Interval.of method has a cache that is not thread safe and can thus return invalid Interval instances. This can manifest for example with
org.antlr.v4.runtime.CommonToken.getText returning invalid tokens, usually either the first character of the input stream or the entire input.
I have seen this happen in the jdbi code here, where sql statement are randomly corrupted when parsed.
It was only observed on arm (aws ec2 running graviton processor).

There are few options how to fix it

1. Make the cache thread safe (will add synchronization overhead)
2. Drop the cache altogether (potential very small object creation overhead, could also not be there if inilining + escape analyzis and scalar replacement works as intended)
3. Initialize the cache with all the values (potential small memory overhead)
4. Use safe idiom for unsafe publication (maybe marking fields final should suffice)

I suggest solution 2. I can prepare PR with the fix.

[sharwell]

that is not thread safe

Can you explain this in detail in the context of the Java memory model? I'm doing some investigation as well to see if I can find details. I want to rule out a (JVM) runtime implementation bug.

[findepi]

Java memory model (JMM) defines what is guaranteed in terms of happens-before relation.
in short

• an action such as write to a field in a thread happens-before any other action done later in the same thread, so eg a subsequent read from the field s guaranteed to return what we wrote there.
• a number of synchronization primitives (many classes in java.util.concurrency, volatile fields, synchronized access) establish happens-before relation between events in different threads

however, the Interval.cache cache (removed in #4902) did not use any synchronization, or any other mechanism that would establish happens-before relationship between one thread writing address of an Interval instance to the cache, and another thread reading that pointer. Therefore, under JMM there was nothing guaranteeing that a thread reading the pointer will also observe a fully initialized object.

[lukasz-stec]

To add to what @findepi said. The issue there was that the reference to the Interval instance could have been visible to other threads before all fields were initialized. According to the JMM FAQ, this would not have been possible if the Interval fields were final.

[stevenschlansker]

With #4903 merged and the cache removed, hopefully this issue can be closed