RFR [9]: default Serialization should issue a fence after reconstructing an Object with final fields
Vitaly Davidovich
vitalyd at gmail.com
Fri Feb 20 14:25:44 UTC 2015
Chris,
I think it's fine if there are intra graph object references during
deserialization - nothing unexpected should happen since intra thread
semantics will be preserved. The freeze action is only a concern once
deserialization completes and user code may publish an object from the
graph unsafely to another thread; that's the only place where the fence is
needed, I think.
sent from my phone
On Feb 20, 2015 6:28 AM, "Chris Hegarty" <chris.hegarty at oracle.com> wrote:
> On 20/02/15 00:27, David Holmes wrote:
>
>> On 20/02/2015 10:13 AM, Vitaly Davidovich wrote:
>>
>>> In addition to Peter's comment, full fence seems unnecessarily strong and
>>> will cause performance issues (especially if the fence is per object
>>> in the
>>> graph). A storeFence should be sufficient here, no?
>>>
>>
>> It should be a fence per graph, or perhaps branches thereof, not per
>> object.
>>
>
> My original intent was to replicate the final-freeze action as performed
> by constructors. What I am hearing is that we do better, without any
> visible side-effect. Since the graph is not published until
> readObject/Unshared returns, the fence can be added there.
>
> That said, it may be possible for one leaf in the graph to reference
> another, but to observe this ( in readObject ), there would have an
> implicit dependency on re-constructor order, which is fragile, at best.
>
> But yes a storeFence (horrible terminology :( ) would suffice given that
>> the freeze action in constructors is only OrderAccess::storestore(). And
>> Unsafe.storeFence() is OrderAccess::release() which is a
>> storestore|storeload barrier.
>>
>
> Thanks, updated.
>
> Updated Webrev:
> http://cr.openjdk.java.net/~chegar/deserialFence/webrev.01/webrev/
>
> Note, the changes in this webrev are overly defensive in the face of
> recursive calls to readObject/Unshared. This should be ok, but probably not
> strictly necessary.
>
> -Chris.
>
>
> David
>>
>>
>>> sent from my phone
>>> On Feb 19, 2015 11:32 AM, "Chris Hegarty" <chris.hegarty at oracle.com>
>>> wrote:
>>>
>>> Additional note ( forgotten from original mail):
>>>>
>>>> The fence is needed for "final-freeze" is a one-off barrier at the
>>>> end of
>>>> deserialization, similar that of constructors . Under normal
>>>> circumstances
>>>> the object being deserialized is not visible until deserialization
>>>> completes, so you don't need a "freeze" until deserialization completes.
>>>>
>>>> -Chris.
>>>>
>>>> On 19 Feb 2015, at 16:25, Chris Hegarty <chris.hegarty at oracle.com>
>>>> wrote:
>>>>
>>>> A number of years ago there was a proposal to use Unsafe.put*Volatile
>>>>>
>>>> methods to set final fields during default deserialisation [1][2],
>>>> but it
>>>> never made it due to concerns about the potential negative impact of the
>>>> additional fences. Now we have a, private, fences API in the platform, I
>>>> think it is time to revisit this.
>>>>
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~chegar/deserialFence/webrev.00/webrev/
>>>>>
>>>>> Note:
>>>>> - Section 17.5.3 in the JLS [3], “Freezes of a final field occur both
>>>>> at the end of the constructor in which the final field is set, and
>>>>> immediately after each modification of a final field via reflection
>>>>> or other special mechanism.” I believe this is a consequence of
>>>>> the way in which setting of final fields is supported in the public
>>>>> API, Field.setAccessible(), ( as defined by JSR 133 ) and should
>>>>> not restrict an implementation from using a more performant
>>>>> means, as is suggested here. The statement in the JLS should
>>>>> be revisited.
>>>>>
>>>>> - Default Serialization already has a dependency on Unsafe, and
>>>>> I don’t see this additional dependency as making that any worse.
>>>>>
>>>>> - Open question, should we include volatile fields as well as final?
>>>>>
>>>>> - The changes in the webrev will issue a fence even if custom
>>>>> deserialization is performed. I think this is ok, as it will be more
>>>>> consuming to try to determine if a custom readObject set a final
>>>>> through reflection, or not.
>>>>>
>>>>> -Chris.
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-6647361
>>>>> [2]
>>>>>
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-
>>>> November/005292.html
>>>>
>>>>
>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-
>>>> December/005456.html
>>>>
>>>> [3]
>>>>>
>>>> http://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.5.3
>>>>
>>>>
>>>>
More information about the core-libs-dev
mailing list