RFR: jsr166 jdk10 integration wave 2

[Martin Buchholz]

On Tue, Aug 8, 2017 at 5:21 AM, Peter Levart <peter.levart at gmail.com> wrote:

> Hi Martin,
>
> Just a purely theoretical question...
>
> On 08/08/2017 02:06 AM, Martin Buchholz wrote:
>
>> Need to fix
>>
>>
>>     1. JDK-8185830 <https://bugs.openjdk.java.net/browse/JDK-8185830>
>>
>> ConcurrentSkipListSet.clone() fails with UnsupportedOperationException
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk10/jsr166-integration/
>>
>> (It would be nice if we could submit a jdk9 backport now instead of
>> waiting)
>>
>
> Regarding ConcurrentSkipListSet.clone()... The 'm' field is final
> presumably to allow "safe" publication of ConcurrentSkipListSet objects to
> other threads via data races.
>
> clone() O.T.O.H. uses Field.setAccessible(true) on a final field, followed
> with Field.set(), which amounts to Unsafe.putObjectVolatile(). Is this
> equivalent to volatile write? Is it possible for a normal write of a
> reference to a cloned ConcurrentSkipListSet (i.e. publication via data
> race) to bubble up before the volatile write of 'm' field inside the clone
> and consequently allow an observer of a reference to the clone to modify
> the backing map of the original ConcurrentSkipListSet?
>

Yeah, I don't think we have any really good answer for this.  I've lobbied
for "safe publication of non-final fields as if final" for a long time.
Even making the field volatile is not enough in theory (but is in practice)
to make the uninitialized field unobservable.

In theory a fence in the constructor is not enough because you need a fence
before every read as well.  Maybe if you replace every write with a release
write and every read with an acquire read you're OK.

But you also want to keep the "final" for software engineering purposes,
since it's "mostly" final.