Hi,

Here are a few comments on logging-1.0.5-alpha1. More to come..

===========

Should the WeakHashtable class be rolled into commons-logging.jar?
It seems easier for users than remembering to deploy the extra jar, and
should be feasable by having something like this in 
   Hashtable foo;
   String version = System.getProperty("java.vm.specification.version");
    if (versionLessThan(version, "1.3")) {
      foo = new Hashtable();
    } else {
      // use reflection to create instance
      foo = createWeakHashtable();
    }
  
Or is the reason for having it separate because there is a performance
hit when using it? If that is so, then file guide.xml should document
that rather than saying "always deploy it when using java 1.3 or later".


===========

The current javadoc for the WeakHashtable class doesn't include a
description of the general problem it's trying to solve (though this is
well described in the guide.xml).

I found it rather difficult to understand the description of the
remaining issue that this class still doesn't handle.

So attached is a proposed patch to the javadoc for the WeakHashtable
class.

BTW, is there a maven command that will actually generate the javadoc
for the optional classes? 

Regards,

Simon
Index: WeakHashtable.java
===================================================================
--- WeakHashtable.java	(revision 156470)
+++ WeakHashtable.java	(working copy)
@@ -22,47 +22,68 @@
 import java.util.*;
 
 /**
- * <p>Implementation of <code>Hashtable</code> that uses <code>WeakReference</code>'s
- * to hold it's keys thus allowing them to be reclaimed by the garbage collector.
- * This class follows the symantics of <code>Hashtable</code> as closely as possible.
- * It therefore does not accept null values or keys.
- * <p>
- * This implementation is also tuned towards a particular purpose: for use as a replacement
- * for <code>Hashtable</code> in <code>LogFactory</code>. This application requires
- * good liveliness for <code>get</code> and <code>put</code>. Various tradeoffs
- * have been made with this in mind.
- * </p>
- * <p>
- * <strong>Usage:</strong> typical use case is as a drop-in replacement 
- * for the <code>Hashtable</code> use in <code>LogFactory</code> for J2EE enviroments
- * running 1.3+ JVMs. Use of this class <i>in most cases</i> (see below) will
- * allow classloaders to be collected by the garbage collector without the need 
- * to call [EMAIL PROTECTED] org.apache.commons.logging.LogFactory#release(ClassLoader) LogFactory.release(ClassLoader)}.
- * </p>
- * <p>
- * In a particular usage scenario, use of <code>WeakHashtable</code> alone will
- * be insufficent to allow garbage collection of a classloader without a call to
- * <code>release</code>.  If the abstract class <code>LogFactory</code> is 
- * loaded by a parent classloader and a concrete subclass implementation of 
- * <code>LogFactory</code> is loaded by a child classloader, the concrete
- * implementation will have a strong reference to the child classloader via the
- * chain <code>getClass().getClassLoader()</code>. The <code>WeakHashtable</code>
- * will have a strong reference to the <code>LogFactory</code> implementation as
- * one of the values in its map. This chain of references will prevent 
- * collection of the child classloader.
- * </p>
- * <p>
- * Such a situation would typically only occur if commons-logging.jar were
- * loaded by a parent classloader (e.g. a server level classloader in a
- * servlet container) and a custom <code>LogFactory</code> implementation were
- * loaded by a child classloader (e.g. a web app classloader).  If use of
- * a custom <code>LogFactory</code> subclass is desired, ensuring that the
- * custom subclass is loaded by the same classloader as <code>LogFactory</code>
- * will prevent problems.  In normal deployments, the standard implementations 
- * of <code>LogFactory</code> found in package <code>org.apache.commons.logging.impl</code> 
- * will be loaded by the same classloader that loads <code>LogFactory</code> 
- * itself, so use of the standard <code>LogFactory</code> implementations
- * should not pose problems.
+ * <p>Implementation of <code>Hashtable</code> that uses <code>WeakReference</code>s
+ * to hold its keys thus allowing them to be reclaimed by the garbage collector.
+ * The associated values are retained using strong references.</p>
+ *
+ * <p>This class follows the symantics of <code>Hashtable</code> as closely as
+ * possible. It therefore does not accept null values or keys.</p>
+ *
+ * <p>Class org.apache.commons.logging.LogFactory looks to see whether this 
+ * class is present in the classpath, and if so then uses it to store
+ * references to the LogFactory classes it loads, rather than using a standard
+ * Hashtable instance. Having this class used instead of Hashtable solves
+ * certain issues related to dynamic reloading of applications in J2EE-style
+ * environments. However this class requires java 1.3 or later, due to its use
+ * of java.lang.ref.WeakReference and related class, and therefore cannot be
+ * included in the main logging distribution which supports JVMs prior to 1.3.
+ * And by the way, this extends Hashtable rather than Hashmap because of 
+ * LogFactory's desire to be compatible with 1.1 JVMs. See the documentation
+ * for method LogFactory.createFactoryStore for more details.</p>
+ *
+ * <p>The reason all this is necessary is due to a classloader issue which
+ * arises when a J2EE-like container includes the library
+ * commons-logging.jar in a classloader that is exposed to components
+ * as a parent of the component-specific classloader.
+ * Each component running in the container gets its own classloader; when
+ * the component loads a LogFactory instance via the component classloader
+ * a reference to it gets stored in the static LogFactory.factories member,
+ * keyed by the component's classloader so different components don't
+ * stomp on each other. When the component is later unloaded, the container
+ * sets the component's classloader to null with the intent that all the 
+ * component's classes get garbage-collected. However there's still a
+ * reference to the component's classloader from the "global" LogFactory's
+ * factories member! One solution is for each component to call
+ * LogFactory.release() when unloaded, but an even nicer one is to use
+ * a Hashmap of weak references. Unfortunately, weak references are
+ * only available in java 1.3+, so this code only uses WeakHashtable if the
+ * class has explicitly been made available on the classpath.</p>
+ *
+ * <p>The implementation of this class is tuned specifically for use with
+ * the LogFactory class.</p>
+ *
+ * <p>Because the presence of this class in the classpath ensures proper
+ * unload of components without the need to call method 
+ * [EMAIL PROTECTED] org.apache.commons.logging.LogFactory#release(ClassLoader) LogFactory.release(ClassLoader)},
+ * it is recommended that this class be deployed along with the standard
+ * commons-logging.jar file when using commons-logging in J2EE
+ * environments (which will presumably be running on Java 1.3 or later).</p>
+ *
+ * <p>There is still one (unusual) scenario in which a component will not 
+ * be correctly unloaded without an explicit release. This is due to the
+ * fact that this class uses weak references for its keys, but strong references
+ * for its values. If the abstract class <code>LogFactory</code> is 
+ * loaded by the container classloader but a subclass of 
+ * <code>LogFactory</code> [LogFactory1] is loaded by the component's 
+ * classloader and an instance stored in the static map associated with the
+ * base LogFactory class, then there is a strong reference from the LogFactory
+ * class to the LogFactory1 instance (as normal) and a strong reference from
+ * the LogFactory1 instance to the component classloader via
+ * <code>getClass().getClassLoader()</code>. To avoid this scenario, ensure
+ * that any custom LogFactory subclass is loaded by the same classloader as 
+ * the base <code>LogFactory</code>. Creating custom LogFactory subclasses is,
+ * however, rare. The standard LogFactoryImpl class should be sufficient
+ * for most or all users.</p>
  * 
  * @author Brian Stansberry
  */

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to