[12] RFR: 8210142: java.util.Calendar.clone() doesn't respect sharedZone flag

[Roger Riggs]

Looks good, thanks for the update.

Roger


On 9/5/18 5:10 PM, naoto.sato at oracle.com wrote:
> 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
>>>>>>
>>>>
>>