Shouldn't DefaultBuildPluginManager#getPluginRealm be synchronized?
If two threads arrive at DefaultBuildPluginManager line 170 at about the
same time, they both will see null pluginRealm and both will then call
setupPluginRealm, which I believe will likely result in the two threads
using two different classloaders.
Also, I still do not believe there is anything wrong with
design/implementation of DefaultClassRealmManager. If it is asked to
createPluginRealm multiple times, it creates multiple realms. Doing
otherwise will result in the same realm being reused for dirrefent
project/plugin combos and will mess up classloading unless entire
classloading approach is reworked.
https://cwiki.apache.org/MAVEN/maven-3x-class-loading.html
--
Regards,
Igor
On 12-07-26 1:27 PM, Kristian Rosenvold wrote:
I have some very specific logs in an issue
(http://code.google.com/p/maven-svn-revision-number-plugin/issues/detail?id=16#c85)
indicating that to class realms are being created for the same plugin
when there is only one execution.
The behaviour seems totally consistent with hitting the catch block at
line 100 of DefaultClassRealmManager, down to
the suffix of the second class realm. Logically, this will happen at
any time when the FIRST construction of the class realm happens in two
parallel modules in the build. Imagine that you have two modules at
the end of the build, both initializing the war plugin. If they are
siblings in the parallel scheduling, the likelyhood of these two
requesting realm in parallel is extremely high.
The thread scheduling will ensure that two such modules will arrive at
DefaultBuildPluginManager line 170 at more or less the same instant.
Both will go through the if test, since this is the first time the
realm is created. The interesting thing is that it seems to me like
the cache key at line 304 must somehow be different, since the
setupPluginRealm method is synchronized.
It seems like I need to follow up on why the cache key is different,
because technically it looks like we should be avoiding the second
call to newRealm, meaning I should really be able to leave the funky
logic as-is.
WDYT ?
Kristian
2012/7/26 Igor Fedorenko <[email protected]>:
I have not looked at parallel execution closely and don't fully
understand the usecase, but I am not sure race condition is the code in
DefaultClassRealmManager.
As far as I can tell, DefaultClassRealmManager only provides create*ock
methods, which create new realms and the current implementation looks
proper.
If parallel execution logic needs to be able to create-or-query plugin
realms in thread-safe manner, I think this should be implemented
somewhere at higher levels, where one-plugin-realm-per-plugin-perproject
constraint can be enforced properly.
Again, I do not know parallel execution, so I can be completely wrong
about this.
--
Regards,
Igor
On 12-07-26 12:36 PM, Jason van Zyl wrote:
In that case we may just want to differentiate because the parallel
execution use case is not one that happens inside the IDE. In the case of
CLI execution it can all be handled up front as additional realm
construction beyond the CLI execution isn't going to happen. In an IDE or
shell wouldn't a map of realms keyed by GAV suffice? Sync on write everyone
is free to read?
On Jul 26, 2012, at 12:06 PM, Igor Fedorenko wrote:
This is required in m2e where multiple versions of the same plugin can
coexist for long time in the same container.
--
Regards,
Igor
On 12-07-26 8:44 AM, Kristian Rosenvold wrote:
There is a race condition in parallel builds that occurs related to
this piece of code:
http://maven.apache.org/ref/3.0.4/maven-core/xref/org/apache/maven/classrealm/DefaultClassRealmManager.html#75
The thing is, for some reason, there's a loop that retries the class
realm generation with a random suffix if the class realm already
exists. In a parallel run, there will be multiple threads requesting
the same realm-id, which semantically should map to the same instance
of the class realm.
(Most plugins do not really mind if there's a duplicate class realm
every now and then, but some take it very seriously ;)
I'm tempted to change the semantics of the "newRealm" method to
"getOrCreateRealm", since that seems to be the correct semantics no
matter what. I've tried tracking the origin of the while loop, and it
seems to be very old. Anyone have any idea of what purpose it served ?
Kristian
---------------------------------------------------------------------
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]
Thanks,
Jason
----------------------------------------------------------
Jason van Zyl
Founder & CTO, Sonatype
Founder, Apache Maven
http://twitter.com/jvanzyl
---------------------------------------------------------
The modern conservative is engaged in one of man's oldest exercises in
moral philosophy; that is,
the search for a superior moral justification for selfishness.
-- John Kenneth Galbraith
---------------------------------------------------------------------
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]
