RE: [jboss-dev] Re: Commons-logging upgrade
by Brian Stansberry
jboss-development-bounces(a)lists.jboss.org wrote:
> Brian Stansberry wrote:
<snip>
>>
>> 1.1 is by default more robust. In this situation it detects the
>> incompatibility of the Log interface, but rather than throwing an
>> exception, it starts walking the classloader hierarchy, trying to
>> load the log adapter from the parent of the webapp classloader,
>> continuing to move up the hierarchy until it finds a compatible
>> adapter or no adapter is available. So, it will eventually find the
>> Log4jLogger loaded from server/all/lib, and that is what is returned.
>>
> This still leads to the problem of wanting to load a
> different log4j impl via jcl than that bound to the
> Log4jFactory in the parent class loader. This was the reason
> for the current jcl patch to allow for late binding. There is
> no way to deal correctly with type hierarchies that are
> dynamic without late binding.
>
Yes. I didn't mean to imply the late binding patch no longer had value;
just that allowing the loading of JCL classes from the war removes a
large category of cases where it is needed. I also wanted to hijack the
thread a bit to get into whether the FilteredPackages was still needed.
:-)
>> As for the test, with jcl in the war, the servlet calls Log log =
>> LogFactory.getLog(...) and in that case the Log interface and the
>> adapter are both loaded from the war.
>>
>> Perhaps a better test is to use a jsp instead of a servlet. IIRC, the
>> jsp processing process involves LogFactory.getLog() calls where the
>> Log type is loaded from the war deployer level while the TCCL is the
>> webapp's. I wrote some tests that used jsps and found no problems.
>> Those ended up in the
>> org.jboss.test.classloader.leak.test.ClassloaderLeakTest test case.
>>
>
> The problem was not the web component getting type systems
> confused. It was the web application mbean or some sub-mbean calling
> LogFactory.getLog() and having this conflict with the host
> mbean that was created by the tomcat deployer service. I
> would have to reproduce the original problem to recall
> exactly why the conflict arose.
I can't remember exactly what it was either. I do remember it was
readily apparent. But, with the late-binding patch I don't see how the
FilteredPackages thing does any significant harm, so perhaps leaving
well enough alone is the way to go.
Brian Stansberry
Lead, AS Clustering
JBoss, a division of Red Hat
Ph: 510-396-3864
skype: bstansberry
19 years, 2 months
RE: [jboss-dev] Re: Commons-logging upgrade
by Brian Stansberry
Was the problem basically that the war-deployer-level mbean would call
Log log = LogFactory.getLog(...) when the TCCL was set to the web app
classloader? The Log type would be from conf/jboss-service.xml class
loader level, but JCL would try to load the log adapter from the war.
The Log interface implemented by the log adapter would be incompatible.
JCL would detect this and throw LogConfigurationException.
1.1 is by default more robust. In this situation it detects the
incompatibility of the Log interface, but rather than throwing an
exception, it starts walking the classloader hierarchy, trying to load
the log adapter from the parent of the webapp classloader, continuing to
move up the hierarchy until it finds a compatible adapter or no adapter
is available. So, it will eventually find the Log4jLogger loaded from
server/all/lib, and that is what is returned.
As for the test, with jcl in the war, the servlet calls Log log =
LogFactory.getLog(...) and in that case the Log interface and the
adapter are both loaded from the war.
Perhaps a better test is to use a jsp instead of a servlet. IIRC, the
jsp processing process involves LogFactory.getLog() calls where the Log
type is loaded from the war deployer level while the TCCL is the
webapp's. I wrote some tests that used jsps and found no problems.
Those ended up in the
org.jboss.test.classloader.leak.test.ClassloaderLeakTest test case.
jboss-development-bounces(a)lists.jboss.org wrote:
> What is the point of discovery not being sensitive to the
> caller context? That is more of a j2se singleton behavior
> which is what caused the original problem. Something has
> changed in the jbossweb layer to allow jcl in the war as this
> originally caused conflicts between the war deployer mbean
> using jcl as the conf/jboss-service.xml class loader level,
> and the web app related mbeans loading jcl from the war class
> loader. This combined with web app mbeans being associated
> with mbeans created by the war deployer with discovery
> occurring in the context of two class loaders caused type
> conflicts. I don't want to have to patch jcl, but we need to
> understand why the test now works with jcl in the war.
>
> Brian Stansberry wrote:
>> JCL 1.1 no longer fails with LogConfigurationException if
>> commons-logging.jar is deployed in WEB-INF/lib and the
>> FilteredPackages thing in the tomcat sar isn't used. If you add the
>> jar to the commons-logging.war used in the test and remove
>> org.apache.commons.logging from FilteredPackages, the test passes.
>>
>> You can also place JCL 1.0.4 in WEB-INF/lib and the test will pass.
>>
>> If we don't want to patch JCL, requiring the packaging of
>> commons-logging in the war is an option. Either way, I think
>> filtering the packages is no longer necessary.
>>
>> JCL doesn't late-bind the Log4j class because when it loads
>> Log4jLogger (the wrapper class) it wants to get a CNFE if log4j
>> isn't available. Part of the discovery algorithm.
>>
>
> _______________________________________________
> jboss-development mailing list
> jboss-development(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/jboss-development
19 years, 2 months
RE: [jboss-dev] Re: Commons-logging upgrade
by Brian Stansberry
JCL 1.1 no longer fails with LogConfigurationException if
commons-logging.jar is deployed in WEB-INF/lib and the FilteredPackages
thing in the tomcat sar isn't used. If you add the jar to the
commons-logging.war used in the test and remove
org.apache.commons.logging from FilteredPackages, the test passes.
You can also place JCL 1.0.4 in WEB-INF/lib and the test will pass.
If we don't want to patch JCL, requiring the packaging of
commons-logging in the war is an option. Either way, I think filtering
the packages is no longer necessary.
JCL doesn't late-bind the Log4j class because when it loads Log4jLogger
(the wrapper class) it wants to get a CNFE if log4j isn't available.
Part of the discovery algorithm.
jboss-development-bounces(a)lists.jboss.org wrote:
> The point of the testWarCommonsLoggingLog4jOverrides was to
> validate that the commons logging log4j factory does not bind
> its factory at the server class loader level and prevent
> resolution of the war local configuration from being used. If
> the upgraded commons logging is also not working then I guess
> it does need patching as well, but I thought it was supposed
> to work correctly for this scenario.
>
> The patched commons logging is in cvs.forge.jboss.com:/cvsroot/jboss
> under the module apache/commons-logging.
>
> http://fisheye.jboss.org/browse/JBoss/apache/commons-logging
>
> Dimitris Andreadis wrote:
>> We upgraded commons-logging in Branch_4_2 to v1.1
>> http://jira.jboss.com/jira/browse/JBAS-2823
>>
>> Now I see in the testsuite
>> org.jboss.test.classloader.test.ScopingUnitTestCase failing
>> (testWarCommonsLoggingLog4jOverrides), I believe due to not using a
>> patched commons-logging.
>>
>> I guess we have to patch v1.1 as well, like in here:
>> http://jira.jboss.com/jira/browse/JBAS-3128
>>
>> I looked in the repository put I can't find the sources from the
>> previous patch.
>>
>> Where are they?
Brian Stansberry
Lead, AS Clustering
JBoss, a division of Red Hat
Ph: 510-396-3864
skype: bstansberry
19 years, 2 months