[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