On 10 Jan 2013, at 07:57, Kristian Rosenvold wrote:

> It seems like I made some kind of mistake in my initial analysis, so
> it looks better !
> 
> Used from core, the wagons are consistently created on one thread and
> handed off to another. From the handoff they seem to be confined to
> that thread, which should work.
> 
> Now there is the issue of the @plexus.requirement annotated fields;
> which are wired by plexus (like ScmManager in ScmWagon). Does anyone
> know if the wiring process in plexus is synchronized ?

There's no guarantee that Plexus field injection will be synchronized, so if 
you're constructing an object in one thread and immediately handing it off to 
another thread without any intervening synchronization then you should use 
volatile fields or ideally introduce a synchronized setter+getter in the 
outermost object. But even without injection you'd want to do this if you were 
setting values in one thread and wanting another thread to see them.

One benefit of JSR330 is that you can use constructor injection along with 
final fields which avoids this kind of thread-visibility issue, since they are 
frozen and made visible to other threads when the constructor completes.

PS. you can actually mark a private field as final and still have it injected, 
because the reflection code uses "setAccessible" to break through the 
encapsulation and this also overrides the 'final-ness' as described in 
http://jeremymanson.blogspot.co.uk/2008/07/immutability-in-java-part-3.html 
while still preserving the thread-visibility effect of final - but this is not 
recommended as it obscures the actual intent.

> If not, I should think all the "plexus.requirement" fields in scm really need 
> to
> be volatile, since they are populated post-construct and immediately
> handed off to another thread.
> 
> Kristian
> 
> 
> 2013/1/9 Olivier Lamy <[email protected]>:
>> Wagon are noted "per-lookup".
>> Have a look at DefaultWagonManager in core, this one always do a
>> container.lookup to get a new fresh wagon instance.
>> So normally plugins using WagonManager must not have any issues
>> (except reusing the same wagon instance)
>> 
>> 2013/1/9 Kristian Rosenvold <[email protected]>:
>>> To illustrate the point I'm trying to make:
>>> 
>>> org.apache.maven.wagon.shared.http4.AbstractHttpClientWagon line 240:
>>> 
>>> protected ClientConnectionManager clientConnectionManager = new
>>> SingleClientConnManager();
>>> 
>>> Now read the docs for SingleClientConnManager (annotated as @ThreadSafe!):
>>> 
>>> "Even though this class is thread-safe it ought to be used by one
>>> execution thread only"
>>> 
>>> This "ought to" means that it's probably unwise to handoff such an
>>> instance to a different thread down the line, which we do. So it does
>>> not support serial thread confinement. It makes me wonder about the
>>> @ThreadSafe annotation....
>>> 
>>> Kristian
>>> 
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>> 
>> 
>> 
>> 
>> --
>> Olivier Lamy
>> Talend: http://coders.talend.com
>> http://twitter.com/olamy | http://linkedin.com/in/olamy
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to