On Mon, 20 Feb 2023 17:03:33 GMT, Peter Levart <plev...@openjdk.org> wrote:
>> We don't fear calling the factory twice for benign races, as the distinct >> constructor factory instances are behaviorally the same. >> >> The true issue lies in the double getfield operations: Java memory model >> doesn't require the second read to happen-after a write reflected in the >> first read, so return this.genericInfo may return null while >> this.genericInfo == null evaluates to false, in case genericInfo is >> initialized lazily by another thread. See >> https://bugs.openjdk.org/browse/JDK-8261404 > > Hi @liach, > > I think @dholmes-ora is worried about the fields in the object being returned > by the getGenericInfo() method and similar. In above case this means fields > in class ConstructorRepository. > I checked it and the entire hierarchy based on > sun.reflect.generics.repository.AbstractRepository with subclasses including > ConstructorRepository is modeled such that all fields are either: > - volatile and lazily initialized; or > - final and initialized in constructor > > Such objects may be published via data race and still be seen consistent on > the accepting side. I don't have any issue with this version. Making the fields volatile is _currently_ unnecessary to ensure correctness -- thanks @plevart for double-checking that all fields in the hierarchy is either `volatile` or `final` -- but adding it is relatively benign (very minor performance cost to a likely-not-very-performance-sensitive code path), reduces fragility and might avoid extraneous allocations of under race conditions. ------------- PR Comment: https://git.openjdk.org/jdk/pull/12643#issuecomment-1572275936
