tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1042421 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Sun, 05 Dec 2010 19:22:20 GMT
Author: markt
Date: Sun Dec  5 19:22:19 2010
New Revision: 1042421

URL: http://svn.apache.org/viewvc?rev=1042421&view=rev
Log:
Update patch.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1042421&r1=1042420&r2=1042421&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sun Dec  5 19:22:19 2010
@@ -204,32 +204,40 @@ PATCHES PROPOSED TO BACKPORT:
 
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50201
   Handle ROOT webapp redployment, host start/stop etc for default access log
-  http://people.apache.org/~markt/patches/2010-12-02-bug50201-tc6.patch
+  http://people.apache.org/~markt/patches/2010-12-05-bug50201-tc6.patch
   +1: markt
   -1: kkolinko:
   1) A typo: in "checkHost = ((ContainerBase) context).started;"
-   should be s/checkHost/checkContext/
+     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.
+     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?).
+     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).
+     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.
+     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.
+     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
 
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48973
   Avoid creating file SESSIONS.ser if there's no session.



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


Mime
View raw message