logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Endre StĂžlsvik <En...@Stolsvik.com>
Subject Re: [COMPATIBILITY] DOMConfigurator
Date Wed, 04 Jan 2006 09:48:25 GMT
On Tue, 3 Jan 2006, Mark Womack wrote:

| If I understand you right, this is what I am planning to do.
| 
| The new Plugin/Watchdog functionality splits the whole "watch" stuff
| into a different heirarchy of classes that perform the watching stuff.
|  In the process it becomes much more flexible and controllable and can
| now "watch" lots of different types of sources than just files.

This sounds good.

| 
| The old configureAndWatch methods are going to be re-implemented to
| use the new FileWatchdog class and the methods themselves will be
| marked as deprecated.

I'd prefer that they _doesn't_ fire any thread, but just simply 
configure-once, and return. The side-effect is that the user's "watching" 
won't work as before, but in return the API is now cleansed from 
thread-forking methods (wouldn't it be?). Given that the 
IMPORTANT-CHANGES.txt file mentions this, and that the method still mostly 
work as before (it does configure the system and makes it run!), this 
wouldn't pose a big problem.

People mostly won't drop-in replace the 1.2.x with 1.3 in existing 
already-running-fine installations where they can't change the code, at 
least that's my 99%-hunch. And if they can change the source-code, they 
_should_ use the new way anyway.

Your suggestion is of course OK, but I'd personally prefer _real cleaning_ 
of the full system, as opposed to scaffolding and patching and thus 
still-existing "bad methods" in the system. The guy that actually drop-in 
replace the 1.3 would maybe even be happy that his system isn't leaking 
threads and memory anymore too!

|  The static configureAndWatch methods will only allow one FileWatchdog 
| at a time, which will shutdown gracefully when log4j is shutdown (part 
| of the new Plugin code).  There is a nasty bug in 1.2 where you can 
| inadvertently set up multiple watch threads and have no way of ever 
| shutting them down.

That the NDC have to be "removed" from Threads, I really didn't know 
untill a couple of weeks ago. I do believe that I'm (at least) an average 
java-coder (,-)) , and I've made such an mistake for 4-5 years. Such 
things are _shocking_.

I want all libraries to clearly tell me what's going on, be "obvious", and 
not fire up any threads or other memory-leaking crap that I won't 
understand the first time I check over the new API, and won't remember 
when I am hit by a totally un-understandable problem way down the road.

For the NDC-case, the mentioning of the important fact that you have to 
remove the context, comes at the 20'th line of the class javadoc. Such a 
thing should be the _first_ line, so that it actually shows in the 
one-line package overview, and be mentioned on _all_ methods of the NDC: 
It is just so very important!! Or the class could be made so that this 
wouldn't be a problem, e.g. use ThreadLocals (which I believe wouldn't 
have the same problem, and would very much faster, and would still work 
on 1.2).

Btw, "Shutting log4j down": Where is this mentioned? If it is important, 
for example for Tomcat reloading of webapps or similar, is should be 
mentioned _everywhere_, really.

| 
| So, the basic functionality will be preserved (with some improvements)

That functionality is not (in my not-very-humble opinion, maybe) 
necessarily something to preserve, but rather a "bug" that should be 
exterminated.


| and pointers to the new code will be provided.

good, good!

Regrads,
Endre

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message