« Return to Thread: concurrency issue with Factory?

Re: concurrency issue with Factory?

by Joe Walker-3 :: Rate this Message:

Reply to Author | View in Thread


That's fine, Tim - go for it.
Joe.

On Fri, Sep 5, 2008 at 8:36 PM, tpeierls <tim@...> wrote:

I see that Joe addressed this by making the builder field volatile.

There's a slight improvement that could be made, however. As the code
stands, the builder field is accessed twice in each get method, once to
check for nullity and then again to use the non-null value.

It would better to read the volatile field once into a temporary variable.
It's more efficient and it prevents the (extremely unlikely) case where the
value is set to null after being seen as non-null.

I have the patch ready to check in, if you like, but it's just adding this
line (in two methods) before the nullity test:

   Builder<T> builder = this.builder;

--tim


tpeierls wrote:
>
> I don't know enough about how the Factory class is used, but if there's
> any possibility that a thread other than the one that calls setBuilder
> will try to read the value of the builder field, then you need to either
> make builder volatile or guard it with some sort of synchronization.
> Otherwise that reader thread might see a null value even if the read
> happens a long time after the call to setBuilder.
>
> I only noticed this because Patrick Lightbody mentioned the existence of
> ServerContextFactory.get() with no arguments -- something I hadn't known
> about -- and I wanted to see how it worked.
>
> --tim
>
>

--
View this message in context: http://www.nabble.com/concurrency-issue-with-Factory--tp19243768p19338068.html
Sent from the DWR - Dev mailing list archive at Nabble.com.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...


 « Return to Thread: concurrency issue with Factory?