tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
Subject svn commit: r1055232 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Tue, 04 Jan 2011 22:53:26 GMT
Author: markt
Date: Tue Jan  4 22:53:25 2011
New Revision: 1055232



Modified: tomcat/tc6.0.x/trunk/STATUS.txt
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Tue Jan  4 22:53:25 2011
@@ -204,59 +204,8 @@ PATCHES PROPOSED TO BACKPORT:
 * Fix
   Handle ROOT webapp redeployment, host start/stop etc for default access log
-  +1: markt
-  -1: kkolinko:
-  1) A typo: in "checkHost = ((ContainerBase) context).started;"
-     should be s/checkHost/checkContext/
-     markt: Fixed in updated patch above
-  2) Possible NPEs in StandardEngine.logAccess(). With the old code
-     the defaultAccessLog field can be checked once, and we know that
-     if it is already not-null it will never become null. That is no
-     longer the case and subsequent accesses to that volatile field can result
-     in NPE. One needs to keep a local copy of that field.
-     markt: Fixed in updated patch above
-  3) addPropertyChangeListener(l). If we are only interested in "defaultHost"
-     property change, shouldn't we add the listener to StandardEngine?
-     I think that there is no need to add it to Context or Host (and
-     are they notified about that change at all?).
-     markt: Fixed in updated patch above
-  4) Initializing defaultAccessLog may be performed by several threads
-     concurrently. That may result in several listener instances being
-     added. (It won't break their operation, but it is just a waste).
-     markt: Agreed it is a waste. Can't do much about it without lots and lots
-            of complexity.
-  5) in AccessLogListener.containerEvent():
-     Context context = (Context) event.getData();
-     Maybe it'd be better to add an instanceof check before that cast.
-     markt: Not worth it. That object has to be an instance of Context else all
-            sorts of stuff would break way before the code got this far.
-  6) In the proposed patch the Listener is only installed iff there
-     is an access log there. It should be installed unconditionally.
-     E.g., if Tomcat starts without access logs, NoopAccessLog is created.
-     As there are no listeners, any change to the configuration
-     (e.g. redeploying the ROOT webapp) will pass unnoticed
-     and won't reenable the access log.
-     markt: Fixed in updated patch above
-  kkolinko: (Re: markt's 2010-12-05-bug50201-tc6.patch)
-  7) Why "Lifecycle.DESTROY_EVENT.endsWith(type)" in a condition?
-     s/endsWith/equals/
-     markt: Auto-complete snafu - fixed in trunk
-  8) I do not see how the listener can be reused when recalculation
-     happens.
-     (We are now installing a new listener when NoAccessLog wins,
-     so my understanding is that it is no more necessary to wait for
-     reuse there. The new listener will catch all the necessary events).
-     Thus all events that force recalculation should result
-     in disabling the listener.
-  9) I propose to use an AtomicReference to guard against registering
-     duplicate listeners (a fix for 4)). See the patch.
-  10) I added install() and uninstall() method to AccessLogListener
-      to help register/unregister the listeners.
-      8),9),10) were applied to trunk in r1045003
-  +1: kkolinko
+  +1: kkolinko, markt
 * Fix

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

View raw message