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

[Roger Riggs]

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
>>>>
>>