Hi again, After two successful complete test suite runs (apart from tests that failed in the same way before) I just committed this.
But even though I saw no problems in the tests, when looking over the code again, I see a possible race with lookup and classloader removal. I think I can fix this by adding some explicit version number that is incremented on classloader removal. A side effect of this will probably be that the set classLoaders will be the only one that has to be copied, thus making the classloader removal a bit faster again. Just to let you know I am onto it. The current commit seems to run fine, and fixes my problem. I'll do another commit if a few hours of testing is ok with the fix I am going to make for this (theoretical) race. Best Regards, Ole Husgaard. Ole Husgaard wrote: > > Hi, > > I just rewrote the locking in ServiceLibraries.java so that > no locks are held while calling the classloaders. > > Externally this file will have the same API and semantics, > but internally it is a bit different: > > When adding or removing classloaders, the maps and sets > that are changed are no longer simply changed. Instead a > copy is made, and the copy is changed. This makes adding > and removing classloaders here a bit slower, but enables > us to call the classloaders without holding any locks. > > When looking up a class or resource, synchronization now > happen twice: Before calling the classloaders, the simple > "do we already have it?" check is done, and if the class > is not found here, the references to the maps and sets > used afterwards are copied to local variables. The rest > of the two lookup methods are pretty much the same, except > that they run without synchronization until a class or > resource is found. If a class or resource is found, > adding it to the local maps is done in another synchronized > block. > If another thread concurrently removes a class loader > while one of the lookup methods call the class loaders with > no synchronization, new copies of the maps are created, > but the lookup thread will continue to work on the old > maps that will later be garbage collected. This way we > avoid ending up with the maps holding a class or resource > from a class loader that has been removed. > > A nice side effect of this is that now all the maps and > sets can be unsynchronized, since the needed synchronization > is already being done at a higher level. > > This is all a bit hairy, but the slowdown is almost only > with the addition and removal of classloaders, and I don't > expect that to happen often. > > Testing the code change as I write this. > > Best Regards, > > Ole Husgaard. > > "Jung , Dr. Christoph" wrote: > > > > Hi there, > > > > In order to verify whether our hypothesis about that ugly deadlocking > > behaviour of the RH classloaders is right > > > > (Sascha, Simone and Ole seem to have some serious problems in that > > direction, if I understood their postings right), > > > > could you please NOT try out the following patch.jar (the synchronized > > modifier has been removed at Class > > java.lang.ClassLoader.loadClassInternal(String), which is IMHO a totally > > uncritical) which violates the > > Sun Binary Code License! > > > > Hence, do NOT start jboss using java -Xbootclasspath/p:patch.jar ... > > > > CGJ > > > > <<patch.jar>> > > > > ------------------------------------------------------------------------ > > Name: patch.jar > > patch.jar Type: Java Archive (application/java-archive) > > Encoding: base64 > > _______________________________________________ > Jboss-development mailing list > [EMAIL PROTECTED] > https://lists.sourceforge.net/lists/listinfo/jboss-development _______________________________________________ Jboss-development mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/jboss-development
