[12] RFR: 8210142: java.util.Calendar.clone() doesn't respect sharedZone flag
naoto.sato at oracle.com
naoto.sato at oracle.com
Wed Sep 5 21:10:45 UTC 2018
Hi Roger,
I updated the fix to share the zone only if the sharedZone is true. Here
is the updated webrev:
http://cr.openjdk.java.net/~naoto/8210142/webrev.01/
Naoto
On 9/5/18 7:07 AM, Roger Riggs wrote:
> Hi Naoto,
>
> That sounds like a test issue. I would not expect a cloned Calendar or
> TimeZone to have a different hashCode.
> None of the fields involved in the hashCode/equals should be different.
> (Or I'm missing something about them).
>
> Thanks, Roger
>
>
> On 9/4/18 5:19 PM, Naoto Sato wrote:
>> Hi Roger,
>>
>> I tentatively tried to return a shared zone within a cloned Calendar.
>> One test failed: java/util/Calendar/CalendarRegression.Test4136399,
>> where it makes sure that the cloned Calendar object hash code should
>> be different for the better distribution. Although the comment does
>> not reflect the current implementation (getTimeZone() returning cloned
>> zone), the intention here seems to have the better hash distribution
>> for cloned objects.
>>
>> Naoto
>>
>> On 9/4/18 1:41 PM, Roger Riggs wrote:
>>> Hi Naoto,
>>>
>>> That access via reflection is going to go away sometime; so I'm not
>>> too concerned about
>>> maintaining compatibility of the internal implementation.
>>> I think I'd rather see the memory savings, however small.
>>> Let see if anyone else has a recommendation.
>>>
>>> Roger
>>>
>>>
>>> On 9/4/18 4:12 PM, Naoto Sato wrote:
>>>> Hi Roger,
>>>>
>>>> Yes, I considered that too, but did not change the behavior and just
>>>> to maintain the field consistent. I agree that it would not be
>>>> observable via the public Calendar API, but some apps (like how the
>>>> submitter found it) may be doing something using reflection.
>>>>
>>>> Naoto
>>>>
>>>> On 9/4/18 12:31 PM, Roger Riggs wrote:
>>>>> Hi Naoto,
>>>>>
>>>>> The spec for clone doesn't say whether the clone should share or
>>>>> not share the TimeZone.
>>>>> Did you consider that if sharedZone was true , *not* to clone the
>>>>> TimeZone?
>>>>> It would still get cloned when requested from getTimeZone().
>>>>>
>>>>> This does seem somewhat safer not to change the cloning behavior
>>>>> but I don't think the behavior would be observable.
>>>>>
>>>>> The current code and test is fine, except for reducing the
>>>>> potential for sharing the TimeZone.
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>> On 9/4/2018 2:14 PM, Naoto Sato wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review the fix to the following issue:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8210142
>>>>>>
>>>>>> The proposed fix is located at:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~naoto/8210142/webrev.00/
>>>>>>
>>>>>> The fix is a simple one line change, which is to make the
>>>>>> sharedZone field consistent with the cloned TimeZone instance in
>>>>>> Calendar.clone().
>>>>>>
>>>>>> Naoto
>>>>>
>>>
>
More information about the core-libs-dev
mailing list