commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <>
Subject Re: logging: WeakHashtable
Date Thu, 18 Nov 2004 21:03:48 GMT
On 18 Nov 2004, at 08:51, Brian Stansberry wrote:

> --- robert burrell donkin
> <> wrote:


>> this means that the value is (effectively) held by a
>> hard reference
>> whilst the key is held by a weak. this should
>> address the narrow issue
>> of the most typical use case. i think that probably
>> the code could be
>> improved by eliminating some unnecessary complexity
>> but that'll have to
>> wait till tomorrow.
> Actually, I think it should work with the
> "commons-logging in WEB-INF/lib" case as well.  In
> that use case, the relevant LogFactory and
> WeakHashtable are actually loaded by the webapp
> classloader, so they too are gc'ed when the webapp is
> undeployed.  Attached is a patch to LogFactoryTest
> that I believe shows this.  Please let me know if this
> is wrongheaded in some way -- I wouldn't be surprised
> at all if it is.

no, that's right. (i thought this one through a while ago but it had 
slipped my mind...)

> I took some time to study what the existing LoadTest
> class does and tried to mimic it to test different
> classloading configurations.  (In fact, if this patch
> is accepted down the road I should do a quick refactor
> to avoid duplication w/ LoadTest).
> The other day when I thought it wouldn't work in the
> WEB-INF/lib situation, my test classloader was
> delegating all loading to its parent EXCEPT
> LogFactoryImpl.  LogFactory and everything else was
> loaded by the parent.  That's not the same as putting
> commons-lib in WEB-INF/lib.  The classloader in the
> attached patch is a more correct mock of a webapp
> classloader.  My mistake the other day does show one
> area where WeakHashtable will fail -- if a custom
> subclass of LogFactory were deployed in WEB-INF/lib
> but commons-logging.jar were on the server classpath.
> But I expect that's a pretty small use case.

i suspect this extends to pretty much anyone who uses a custom 
LogFactory implementation where commons-logging is in a parent 
classloader and the implementation is deployed in the child.

custom LogFactory implementations are not very useful at the moment and 
so i'd be happy just to live with a note in the documentation about 
this limitation.

in addition, this use case will be addressed very well by the bytecode 
stuff. (the idea is that instead of discovering a log factory at 
runtime, all the calls will be rewired when the classes are enhanced.) 
if you're deploying a stand alone web-app with a definite need to use a 
particular LogFactory, it's more reliable to dope all the jar's than to 
rely on discovery.

> Oh, BTW, the patch also removes the previous test of
> whether the LogFactory is eventually released once the
> classloader is released.  Now it just tests if the
> classloader is released.  Testing release of the
> LogFactory basically duplicated what was already done
> in WeakHashtableTest.

i'll look forward to see your patch (either i've missed it, i'm 
confused or it was stripped this time...)

i'll leave my tidy up for a few days (give you a chance to get patching 
without me treading on your toes). once everyone's happy with the 
class, i plan to start pushing towards a 1.0.5 release. it'll probably 
be release from a branch so that the release candidate for long enough.

>> one interesting feature of garbage collectors (which
>> foxed me for a
>> while) is that there doesn't seem to be any
>> guarantee as to when the
>> reference is placed onto the queue. (at least i
>> can't find one: if
>> anyone knows different please jump in.) on the
>> (macOSX) JVM i use, it
>> appears that the reference is placed onto the queue
>> late enough to
>> cause one of the tests to fail. placing a hard loop
>> that polls for the
>> released reference to be placed on another queue
>> results in the test
>> passing...

> Interesting.  This whole thing's been interesting --
> way more than I thought it would be when I started off
> grunting through a bunch of reflection :)

+1 :)

- robert

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message