ISODateTimeFormat thread safety

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ISODateTimeFormat thread safety

Lin Wang
The javadoc of ISODateTimeFormat class says it's thread-safe and immutable. The static fields are lazily initialized. However it seems it's not done in a thread-safe manner.

For example, dt is lazily initialized in
    public static DateTimeFormatter dateTime() {
        if (dt == null) {
            dt = new DateTimeFormatterBuilder()
                .append(date())
                .append(tTime())
                .toFormatter();
        }
        return dt;
    }

When there are two threads both inside this method, is it possible that one thread sees an unsafely published non-null dt value due to cache incoherence?

Did I miss something?

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ISODateTimeFormat thread safety

jodastephen
You may be right, although neither volatile nor synchronized are appealing here.
Please raise an issue on GitHub
Stephen

On 12 July 2013 10:19, Lin Wang <[hidden email]> wrote:

> The javadoc of ISODateTimeFormat class says it's thread-safe and immutable.
> The static fields are lazily initialized. However it seems it's not done in
> a thread-safe manner.
>
> For example, dt is lazily initialized in
>     public static DateTimeFormatter dateTime() {
>         if (dt == null) {
>             dt = new DateTimeFormatterBuilder()
>                 .append(date())
>                 .append(tTime())
>                 .toFormatter();
>         }
>         return dt;
>     }
>
> When there are two threads both inside this method, is it possible that one
> thread sees an unsafely published non-null dt value due to cache
> incoherence?
>
> Did I miss something?
>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Joda-interest mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/joda-interest
>

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ISODateTimeFormat thread safety

Roger Riggs
I don't see a functional issue with this;
The DateTimeFormatter created by each thread will have exactly the same
behavior and semantics.
The caller gets an instance that preforms as required.

dt is just a cache to improve performance.

The only condition at risk is the two calls to dateTime() do not return
the same (==) instance.

 From the callers point of view what other observable behavior might be
different?

Roger



On 7/12/2013 5:27 AM, Stephen Colebourne wrote:

> You may be right, although neither volatile nor synchronized are appealing here.
> Please raise an issue on GitHub
> Stephen
>
> On 12 July 2013 10:19, Lin Wang <[hidden email]> wrote:
>> The javadoc of ISODateTimeFormat class says it's thread-safe and immutable.
>> The static fields are lazily initialized. However it seems it's not done in
>> a thread-safe manner.
>>
>> For example, dt is lazily initialized in
>>      public static DateTimeFormatter dateTime() {
>>          if (dt == null) {
>>              dt = new DateTimeFormatterBuilder()
>>                  .append(date())
>>                  .append(tTime())
>>                  .toFormatter();
>>          }
>>          return dt;
>>      }
>>
>> When there are two threads both inside this method, is it possible that one
>> thread sees an unsafely published non-null dt value due to cache
>> incoherence?
>>
>> Did I miss something?
>>
>> ------------------------------------------------------------------------------
>> See everything from the browser to the database with AppDynamics
>> Get end-to-end visibility with application monitoring from AppDynamics
>> Isolate bottlenecks and diagnose root cause in seconds.
>> Start your free trial of AppDynamics Pro today!
>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Joda-interest mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/joda-interest
>>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Joda-interest mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/joda-interest


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ISODateTimeFormat thread safety

Nils Kilden-Pedersen
On Fri, Jul 12, 2013 at 8:05 AM, roger riggs <[hidden email]> wrote:
I don't see a functional issue with this;

You need to study the Java memory model then.
 
The DateTimeFormatter created by each thread will have exactly the same
behavior and semantics.
The caller gets an instance that preforms as required.

No, that's not guaranteed with the current code.
 

dt is just a cache to improve performance.

The only condition at risk is the two calls to dateTime() do not return
the same (==) instance.

No, the risk is that a second concurrent thread gets a reference to the not yet completed object, which can cause unpredictable behavior.
 

 From the callers point of view what other observable behavior might be
different?

It's a possible race condition and the memory model allows re-ordering of operations, so anything could go wrong, from an Exception to plain wrong formatting.
 

Roger



On 7/12/2013 5:27 AM, Stephen Colebourne wrote:
> You may be right, although neither volatile nor synchronized are appealing here.
> Please raise an issue on GitHub
> Stephen
>
> On 12 July 2013 10:19, Lin Wang <[hidden email]> wrote:
>> The javadoc of ISODateTimeFormat class says it's thread-safe and immutable.
>> The static fields are lazily initialized. However it seems it's not done in
>> a thread-safe manner.
>>
>> For example, dt is lazily initialized in
>>      public static DateTimeFormatter dateTime() {
>>          if (dt == null) {
>>              dt = new DateTimeFormatterBuilder()
>>                  .append(date())
>>                  .append(tTime())
>>                  .toFormatter();
>>          }
>>          return dt;
>>      }
>>
>> When there are two threads both inside this method, is it possible that one
>> thread sees an unsafely published non-null dt value due to cache
>> incoherence?
>>
>> Did I miss something?
>>
>> ------------------------------------------------------------------------------
>> See everything from the browser to the database with AppDynamics
>> Get end-to-end visibility with application monitoring from AppDynamics
>> Isolate bottlenecks and diagnose root cause in seconds.
>> Start your free trial of AppDynamics Pro today!
>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Joda-interest mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/joda-interest
>>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Joda-interest mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/joda-interest


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: ISODateTimeFormat thread safety

Roger Riggs
Hi,

I read the code that the assignment is of the value returned by "toFormatter()".
In this "fluent" style each of the intermediates is an instance on which the next method is executed.

The assignment is not done until after 'toFormatter()' completes.

Roger

On 7/12/2013 9:37 AM, Nils Kilden-Pedersen wrote:
On Fri, Jul 12, 2013 at 8:05 AM, roger riggs <[hidden email]> wrote:
I don't see a functional issue with this;

You need to study the Java memory model then.
 
The DateTimeFormatter created by each thread will have exactly the same
behavior and semantics.
The caller gets an instance that preforms as required.

No, that's not guaranteed with the current code.
 

dt is just a cache to improve performance.

The only condition at risk is the two calls to dateTime() do not return
the same (==) instance.

No, the risk is that a second concurrent thread gets a reference to the not yet completed object, which can cause unpredictable behavior.
 

 From the callers point of view what other observable behavior might be
different?

It's a possible race condition and the memory model allows re-ordering of operations, so anything could go wrong, from an Exception to plain wrong formatting.
 

Roger



On 7/12/2013 5:27 AM, Stephen Colebourne wrote:
> You may be right, although neither volatile nor synchronized are appealing here.
> Please raise an issue on GitHub
> Stephen
>
> On 12 July 2013 10:19, Lin Wang <[hidden email]> wrote:
>> The javadoc of ISODateTimeFormat class says it's thread-safe and immutable.
>> The static fields are lazily initialized. However it seems it's not done in
>> a thread-safe manner.
>>
>> For example, dt is lazily initialized in
>>      public static DateTimeFormatter dateTime() {
>>          if (dt == null) {
>>              dt = new DateTimeFormatterBuilder()
>>                  .append(date())
>>                  .append(tTime())
>>                  .toFormatter();
>>          }
>>          return dt;
>>      }
>>
>> When there are two threads both inside this method, is it possible that one
>> thread sees an unsafely published non-null dt value due to cache
>> incoherence?
>>
>> Did I miss something?
>>
>> ------------------------------------------------------------------------------
>> See everything from the browser to the database with AppDynamics
>> Get end-to-end visibility with application monitoring from AppDynamics
>> Isolate bottlenecks and diagnose root cause in seconds.
>> Start your free trial of AppDynamics Pro today!
>> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Joda-interest mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/joda-interest
>>
> ------------------------------------------------------------------------------
> See everything from the browser to the database with AppDynamics
> Get end-to-end visibility with application monitoring from AppDynamics
> Isolate bottlenecks and diagnose root cause in seconds.
> Start your free trial of AppDynamics Pro today!
> http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
> _______________________________________________
> Joda-interest mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/joda-interest


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest



------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk


_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest


------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
Joda-interest mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/joda-interest
Loading...