jackrabbit-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Meschberger <fmesc...@gmail.com>
Subject Re: Extending SimpleWebDavServlet
Date Mon, 29 Sep 2008 07:10:09 GMT
Hi Marc,

You are getting me on my left foot ;-)

I am neither an export on any Memory Model issue, but I always thought
that the construct

     fieldX = new SomeType(zzz);

would be ok, because fieldX would only be set after the SomeType
instance has properly been setup and ready for use.

Have you encountered a concrete problem with this or is a pure academic
discussion ?

Regards
Felix

Marc Speck schrieb:
> Thanks for the replies, Alex and Felix.
> 
> 
> 
>> The original author is
>> currently on vacation, maybe she can answer you later.
>>
> Sure, I've just done the same.
> 
> Before giving my reply, I want to emphasize that I'm on thin ice when
> talking about memory model in Java 1.5 let alone 1.4 that Jackrabbit
> requires. Multi-threading in Java is next to black magic to me, so I rather
> cite below the sources of information.
> 
> 
> 
>>> public DavLocatorFactory getLocatorFactory() {
>>>     if (locatorFactory == null) {
>>>         locatorFactory = new LocatorFactoryImplEx(resourcePathPrefix);
>>>     }
>>>     return locatorFactory;
>>> }
>>>
>>>
>>> I just ask why this is thread safe. It is not called in servlet.init() or
>>> the like, isn't it?
>> I assume the default implementation has no thread issues actually
>> because the LocatorFactoryImplEx is a very simple class with a single
>> internal field set by the constructor.
> 
> 
> Actually, this internal field is the source of a potential data race: thread
> A runs up to the point, when it has constructed LocatorFactoryImplEx but has
> not yet assigned the internal field. Thread A gets on hold relative to
> thread B. B runs through the getter and sees that locatorFactory is not
> null, fetches the object, access the uninitialized internal field and voilĂ !
> We have a mess.
> 
> What's generally confusing to me is that you never know about the variable
> visibility between threads and the relative succession of different threads
> (sometimes called "happens-before"). The lazy initialization problem as
> shown above is very well described in "Java concurency in practice" by Brian
> Goetz under "16.2.1 Unsafe publication".
> 
> 
>> In case of multi-threaded call to the getLocatorFactory method, multiple
>> instances of the LocatorFactoryImplEx class may be created of which only
>> a single one will ultimately remain assigned to the locatorFactory field.
> 
> 
> Yes, that's not a problem and can also happen with the suggestion below.
> 
> 
> 
>> If you overwrite this method and have a more complicated implementation
>> of the DavLocatorFactory interface, which you want to ensure, that only
>> a single instance is created, I suggest you implement synchronization in
>> your overwritten method.
> 
> 
> Agreed but this is not the issue.
> 
> If there is a data race present, other methods like
> SimpleWebdavServlet.getResourceFactory() show the same behavior.
> 
> The best solution by far is not using lazy initialization at all. If you
> still want to delay the initialization time (and spend finally more time
> with all the getters), you could use the single-check idiom as described
> e.g. in Effective Java by Joshua Bloch, 2nd edition, p. 284. This works in
> Java 1.5, I think there is a problem with volatile in 1.4.
> 
> private volatile DavLocatorFactory locatorFactory
> public DavLocatorFactory getLocatorFactory() {
>    DavLocatorFactory result = locatorFactory
>    if (result == null)
>       locatorFactory = result = new
> LocatorFactoryImplEx(resourcePathPrefix);
>    return result
> }
> 
> Synchronizing the getter helps also. This is done by some getters in
> SimpleWebdavServlet.
> 
> I'm happy to learn what's wrong with my line of thoughts about
> multi-threading...
> Marc
> 


Mime
View raw message