Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-21 Thread Venkateswara R Chintala
Thanks Peter for sponsoring this patch. Is there anything else that needs to be done from my end for this patch to be integrated? Please let me know. -Venkat On 14/11/17 8:46 PM, Peter Levart wrote: Hi Venkat, I created the following issue: https://bugs.openjdk.java.net/browse/JDK-8191216

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-15 Thread Peter Levart
Hi David, On 11/14/2017 10:28 PM, David Holmes wrote: Hi Peter, On 15/11/2017 1:02 AM, Peter Levart wrote: Hi David, On 11/11/2017 07:51 AM, David Holmes wrote: AFAICS SimpleTimeZone is simply not thread-safe. It has state that can be modified concurrently without synchronization and with fi

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-14 Thread David Holmes
Hi Peter, On 15/11/2017 1:02 AM, Peter Levart wrote: Hi David, On 11/11/2017 07:51 AM, David Holmes wrote: AFAICS SimpleTimeZone is simply not thread-safe. It has state that can be modified concurrently without synchronization and with fields not even declared volatile. Only the "cache" makes

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-14 Thread Peter Levart
Hi Venkat, I created the following issue: https://bugs.openjdk.java.net/browse/JDK-8191216 I can also sponsor this patch and push it for JDK 10. The patch that you are proposing looks good to me. Does anybody have anything else to say? Regards, Peter On 11/13/2017 11:28 AM, Venkateswara R

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-14 Thread Peter Levart
Hi David, On 11/11/2017 07:51 AM, David Holmes wrote: AFAICS SimpleTimeZone is simply not thread-safe. It has state that can be modified concurrently without synchronization and with fields not even declared volatile. Only the "cache" makes an attempt to use synchronization. So clone() is neve

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-13 Thread Venkateswara R Chintala
Thanks David, Peter for your review and comments. As I am new to the community, can you please help me to open a bug and integrate the changes into code base? -Venkat On 12/11/17 2:19 AM, Peter Levart wrote: Hi David, Venkat, On 11/11/17 21:15, Peter Levart wrote: For example, take the foll

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-11 Thread Peter Levart
Hi David, Venkat, On 11/11/17 21:15, Peter Levart wrote: For example, take the following method: String defaultTZID() {     return TimeZone.getDefault().getID(); } When JIT compiles it and inlines invocations to other methods within it, it can prove that cloned TimeZone instance never escapes

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-11 Thread Peter Levart
Hi David, On 11/11/17 13:37, David Holmes wrote: Hi Peter, On 11/11/2017 8:06 PM, Peter Levart wrote: Hi Venkateswara R Chintala, I would like to remind that TimeZone.clone() is also in the code path of java.time.ZoneId.systemDefault() where it was relied on to be optimized by JIT to never

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-11 Thread David Holmes
Hi Peter, On 11/11/2017 8:06 PM, Peter Levart wrote: Hi Venkateswara R Chintala, I would like to remind that TimeZone.clone() is also in the code path of java.time.ZoneId.systemDefault() where it was relied on to be optimized by JIT to never actually allocate the cloned TimeZone object by em

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-11 Thread Peter Levart
Hi Venkat, On 11/10/17 13:07, Venkateswara R Chintala wrote: Hi, In a multi-threaded environment, when java.util.SimpleTimeZone object is used to create a default timezone, there can be a race condition between the methods java.util.Timezone.getDefault() and java.util.Timezone.getDefaultRef(

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-11 Thread Peter Levart
Hi Venkateswara R Chintala, I would like to remind that TimeZone.clone() is also in the code path of java.time.ZoneId.systemDefault() where it was relied on to be optimized by JIT to never actually allocate the cloned TimeZone object by employing escape analysis. It would be nice to verify if

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-10 Thread David Holmes
AFAICS SimpleTimeZone is simply not thread-safe. It has state that can be modified concurrently without synchronization and with fields not even declared volatile. Only the "cache" makes an attempt to use synchronization. So clone() is never guaranteed to actually produce a copy with valid/cons

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-10 Thread Venkateswara R Chintala
Thanks Sean. I am pasting the patch here: --- old/src/java.base/share/classes/java/util/SimpleTimeZone.java 2017-11-11 11:17:38.643867420 +0530 +++ new/src/java.base/share/classes/java/util/SimpleTimeZone.java 2017-11-11 11:17:38.375870421 +0530 @@ -868,7 +868,11 @@   */ public Object

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-10 Thread Seán Coffey
I think the OpenJDK mailing lists accept attachments if in patch format. If it's a simple short patch, it's acceptable to paste it into email body. Easiest solution is to use webrev[1]. If you can't upload this to cr.openjdk.java.net - then one of your colleagues may be able to help. [1] http

Re: Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-10 Thread Venkateswara R Chintala
Looks like the patch attached earlier is not visible. As this is my first contribution, please let me know how I can send the patch for review. On 10/11/17 5:37 PM, Venkateswara R Chintala wrote: Hi, In a multi-threaded environment, when java.util.SimpleTimeZone object is used to create a de

Incorrect validation of DST in java.util.SimpleTimeZone

2017-11-10 Thread Venkateswara R Chintala
Hi, In a multi-threaded environment, when java.util.SimpleTimeZone object is used to create a default timezone, there can be a race condition between the methods java.util.Timezone.getDefault() and java.util.Timezone.getDefaultRef() which can result in inconsistency of cache that is used to v