tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject svn commit: r919742 - in /tomcat/tc6.0.x/trunk: ./ STATUS.txt java/org/apache/catalina/startup/Catalina.java java/org/apache/juli/ClassLoaderLogManager.java java/org/apache/juli/FileHandler.java webapps/docs/changelog.xml
Date Sat, 06 Mar 2010 11:15:07 GMT
Author: kkolinko
Date: Sat Mar  6 11:15:07 2010
New Revision: 919742

URL: http://svn.apache.org/viewvc?rev=919742&view=rev
Log:
https://issues.apache.org/bugzilla/show_bug.cgi?id=48831
Improve logging shutdown behaviour.
Use Catalina's shutdown hook to shutdown Tomcat and JULI. This enables them to
be shutdown in the correct order.
Do not shutdown global handlers several times. (BZ 48831)

Modified:
    tomcat/tc6.0.x/trunk/   (props changed)
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/Catalina.java
    tomcat/tc6.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java
    tomcat/tc6.0.x/trunk/java/org/apache/juli/FileHandler.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc6.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Sat Mar  6 11:15:07 2010
@@ -1,2 +1,2 @@
 /tomcat:883362
-/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,666232,673796,673820,677910,683969,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,713953,714002,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,752323,753039,757335,757774,758249,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,77
 0809,770876,772872,776921,776924,776935,776945,777464,777466,777576,777625,778379,778523-778524,781528,781779,782145,782791,783316,783696,783724,783756,783762,783766,783863,783934,784453,784602,784614,785381,785688,785768,785859,786468,786487,786490,786496,786667,787627,787770,787985,789389,790405,791041,791184,791194,791224,791243,791326,791328,791789,792740,793372,793757,793882,793981,794082,794673,794822,795043,795152,795210,795457,795466,797168,797425,797596,797607,802727,802940,804462,804544,804734,805153,809131,809603,810916,810977,812125,812137,812432,813001,813013,813866,814180,814708,814876,815972,816252,817442,817822,819339,819361,820110,820132,820874,820954,821397,828196,828201,828210,828225,828759,830378-830379,830999,831106,831774,831785,831828,831850,831860,832214,832218,833121,833545,834047,835036,835336,836405,881396,881412,883130,883134,883146,883165,883177,883362,883565,884341,885038,885231,885241,885260,885901,885991,886019,888072,889363,889606,889716,8901
 39,890265,890349-890350,890417,891185-891187,891583,892198,892341,892415,892464,892555,892812,892814,892817,892843,892887,893321,893493,894580,894586,894805,894831,895013,895045,895057,895191,895392,895703,896370,896384,897380-897381,897776,898126,898256,898468,898527,898555,898558,898718,898836,898906,899284,899348,899420,899653,899769-899770,899783,899788,899792,899916,899918-899919,899935,899949,903916,905020,905151,905722,905728,905735,907311,907513,907538,907652,907819,907825,907864,908002,908721,908754,908759,909097,909206,909212,909525,909636,909869,909875,909887,910266,910370,910442,910471,915226,915737,915861,916097,916141,916157,916170,917598,917633,918093,918684
+/tomcat/trunk:601180,606992,612607,630314,640888,652744,653247,666232,673796,673820,677910,683969,683982,684001,684081,684234,684269-684270,685177,687503,687645,689402,690781,691392,691805,692748,693378,694992,695053,695311,696780,696782,698012,698227,698236,698613,699427,699634,701355,709294,709811,709816,710063,710066,710125,710205,711126,711600,712461,712467,713953,714002,718360,719119,719124,719602,719626,719628,720046,720069,721040,721286,721708,721886,723404,723738,726052,727303,728032,728768,728947,729057,729567,729569,729571,729681,729809,729815,729934,730250,730590,731651,732859,732863,734734,740675,740684,742677,742697,742714,744160,744238,746321,746384,746425,747834,747863,748344,750258,750291,750921,751286-751287,751289,751295,752323,753039,757335,757774,758249,758365,758596,758616,758664,759074,761601,762868,762929,762936-762937,763166,763183,763193,763228,763262,763298,763302,763325,763599,763611,763654,763681,763706,764985,764997,765662,768335,769979,770716,77
 0809,770876,772872,776921,776924,776935,776945,777464,777466,777576,777625,778379,778523-778524,781528,781779,782145,782791,783316,783696,783724,783756,783762,783766,783863,783934,784453,784602,784614,785381,785688,785768,785859,786468,786487,786490,786496,786667,787627,787770,787985,789389,790405,791041,791184,791194,791224,791243,791326,791328,791789,792740,793372,793757,793882,793981,794082,794673,794822,795043,795152,795210,795457,795466,797168,797425,797596,797607,802727,802940,804462,804544,804734,805153,809131,809603,810916,810977,812125,812137,812432,813001,813013,813866,814180,814708,814876,815972,816252,817442,817822,819339,819361,820110,820132,820874,820954,821397,828196,828201,828210,828225,828759,830378-830379,830999,831106,831774,831785,831828,831850,831860,832214,832218,833121,833545,834047,835036,835336,836405,881396,881412,883130,883134,883146,883165,883177,883362,883565,884341,885038,885231,885241,885260,885901,885991,886019,888072,889363,889606,889716,8901
 39,890265,890349-890350,890417,891185-891187,891583,892198,892341,892415,892464,892555,892812,892814,892817,892843,892887,893321,893493,894580,894586,894805,894831,895013,895045,895057,895191,895392,895703,896370,896384,897380-897381,897776,898126,898256,898468,898527,898555,898558,898718,898836,898906,899284,899348,899420,899653,899769-899770,899783,899788,899792,899916,899918-899919,899935,899949,903916,905020,905151,905722,905728,905735,907311,907513,907538,907652,907819,907825,907864,908002,908721,908754,908759,909097,909206,909212,909525,909636,909869,909875,909887,910266,910370,910442,910471,910974,915226,915737,915861,916097,916141,916157,916170,917598,917633,918093,918594,918684,918787,918792,918799,918885

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=919742&r1=919741&r2=919742&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Mar  6 11:15:07 2010
@@ -99,27 +99,6 @@
   +1: kkolinko, markt
   -1:
 
-* Improve logging behaviour when shutdown occurs via a shutdownhook. If present,
-  use Catalina's shutdown hook to shutdown Tomcat and JULI. This enables them to
-  be shutdown in the correct order. 
-  http://svn.apache.org/viewvc?rev=910974&view=rev
-  http://svn.apache.org/viewvc?rev=918594&view=rev (review feedback BZ48831)
-  +1: markt
-  +1: kkolinko: with the additional patches proposed below
-  -1: 
-
-  Additional patches:
-  http://svn.apache.org/viewvc?rev=918792&view=rev (FileHandler: whitespace)
-  http://svn.apache.org/viewvc?rev=918799&view=rev (FileHandler: try/finally)
-  http://svn.apache.org/viewvc?rev=918787&view=rev (ClassLoaderLogManager: fix BZ48831)
-  http://svn.apache.org/viewvc?rev=918885&view=rev (ignore reset call from j.u.l.LogManager.Cleaner)
-
-  here is the combined patch file
-  (revisions 910974,918594,918792,918799,918787,918885):
-  http://people.apache.org/~kkolinko/patches/2010-03-04_tc6_bug48831.patch
-  +1: kkolinko, markt, rjung
-  -1:
-
 * Replace UTF-8 characters with the proper \uxxxx encoding in the French translation.
   That prevents deployment of applications when using french language.
   http://people.apache.org/~rjung/patches/french_properties_utf.patch

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/Catalina.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/Catalina.java?rev=919742&r1=919741&r2=919742&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/Catalina.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/startup/Catalina.java Sat Mar  6 11:15:07
2010
@@ -29,11 +29,13 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.logging.LogManager;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.Lifecycle;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.core.StandardServer;
+import org.apache.juli.ClassLoaderLogManager;
 import org.apache.tomcat.util.digester.Digester;
 import org.apache.tomcat.util.digester.Rule;
 import org.xml.sax.Attributes;
@@ -593,6 +595,15 @@
                     shutdownHook = new CatalinaShutdownHook();
                 }
                 Runtime.getRuntime().addShutdownHook(shutdownHook);
+                
+                // If JULI is being used, disable JULI's shutdown hook since
+                // shutdown hooks run in parallel and log messages may be lost
+                // if JULI's hook completes before the CatalinaShutdownHook()
+                LogManager logManager = LogManager.getLogManager();
+                if (logManager instanceof ClassLoaderLogManager) {
+                    ((ClassLoaderLogManager) logManager).setUseShutdownHook(
+                            false);
+                }
             }
         } catch (Throwable t) {
             // This will fail on JDK 1.2. Ignoring, as Tomcat can run
@@ -617,6 +628,14 @@
             // doesn't get invoked twice
             if (useShutdownHook) {
                 Runtime.getRuntime().removeShutdownHook(shutdownHook);
+
+                // If JULI is being used, re-enable JULI's shutdown to ensure
+                // log messages are not lost
+                LogManager logManager = LogManager.getLogManager();
+                if (logManager instanceof ClassLoaderLogManager) {
+                    ((ClassLoaderLogManager) logManager).setUseShutdownHook(
+                            true);
+                }
             }
         } catch (Throwable t) {
             // This will fail on JDK 1.2. Ignoring, as Tomcat can run
@@ -673,6 +692,13 @@
                 Catalina.this.stop();
             }
             
+            // If JULI is used, shut JULI down *after* the server shuts down
+            // so log messages aren't lost
+            LogManager logManager = LogManager.getLogManager();
+            if (logManager instanceof ClassLoaderLogManager) {
+                ((ClassLoaderLogManager) logManager).shutdown();
+            }
+
         }
 
     }

Modified: tomcat/tc6.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java?rev=919742&r1=919741&r2=919742&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java Sat Mar  6 11:15:07
2010
@@ -50,25 +50,8 @@
         
         @Override
         public void run() {
-            // The JVM us being shutdown. Make sure all loggers for all class
-            // loaders are shutdown
-            for (ClassLoaderLogInfo clLogInfo : classLoaderLoggers.values()) {
-                for (Logger logger : clLogInfo.loggers.values()) {
-                    resetLogger(logger);
-                }
-            }
-        }
-            
-        private void resetLogger(Logger logger) {
-            
-            Handler[] handlers = logger.getHandlers();
-            for (Handler handler : handlers) {
-                logger.removeHandler(handler);
-                try {
-                    handler.close();
-                } catch (Exception e) {
-                    // Ignore
-                }
+            if (useShutdownHook) {
+                shutdown();
             }
         }
 
@@ -105,7 +88,29 @@
      */
     protected ThreadLocal<String> prefix = new ThreadLocal<String>();
 
+
+    /**
+     * Determines if the shutdown hook is used to perform any necessary
+     * clean-up such as flushing buffered handlers on JVM shutdown. Defaults to
+     * <code>true</code> but may be set to false if another component ensures
+     * that {@link #shutdown()} is called.
+     */
+    protected volatile boolean useShutdownHook = true;
+
     
+    // ------------------------------------------------------------- Properties
+
+
+    public boolean isUseShutdownHook() {
+        return useShutdownHook;
+    }
+
+
+    public void setUseShutdownHook(boolean useShutdownHook) {
+        this.useShutdownHook = useShutdownHook;
+    }
+
+
     // --------------------------------------------------------- Public Methods
 
 
@@ -291,7 +296,59 @@
         readConfiguration(is, Thread.currentThread().getContextClassLoader());
     
     }
-        
+
+    @Override
+    public void reset() throws SecurityException {
+        Thread thread = Thread.currentThread();
+        if (thread.getClass().getName().startsWith(
+                "java.util.logging.LogManager$")) {
+            // Ignore the call from java.util.logging.LogManager.Cleaner,
+            // because we have our own shutdown hook
+            return;
+        }
+        ClassLoader classLoader = thread.getContextClassLoader();
+        ClassLoaderLogInfo clLogInfo = getClassLoaderInfo(classLoader);
+        resetLoggers(clLogInfo);
+        super.reset();
+    }
+
+    /**
+     * Shuts down the logging system.
+     */
+    public void shutdown() {
+        // The JVM us being shutdown. Make sure all loggers for all class
+        // loaders are shutdown
+        for (ClassLoaderLogInfo clLogInfo : classLoaderLoggers.values()) {
+            resetLoggers(clLogInfo);
+        }
+    }
+
+    // -------------------------------------------------------- Private Methods
+    private void resetLoggers(ClassLoaderLogInfo clLogInfo) {
+        // This differs from LogManager#resetLogger() in that we close not all
+        // handlers of all loggers, but only those that are present in our
+        // ClassLoaderLogInfo#handlers list. That is because our #addLogger(..)
+        // method can use handlers from the parent class loaders, and closing
+        // handlers that the current class loader does not own would be not
+        // good.
+        synchronized (clLogInfo) {
+            for (Logger logger : clLogInfo.loggers.values()) {
+                Handler[] handlers = logger.getHandlers();
+                for (Handler handler : handlers) {
+                    logger.removeHandler(handler);
+                }
+            }
+            for (Handler handler : clLogInfo.handlers.values()) {
+                try {
+                    handler.close();
+                } catch (Exception e) {
+                    // Ignore
+                }
+            }
+            clLogInfo.handlers.clear();
+        }
+    }
+
     // ------------------------------------------------------ Protected Methods
 
 

Modified: tomcat/tc6.0.x/trunk/java/org/apache/juli/FileHandler.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/juli/FileHandler.java?rev=919742&r1=919741&r2=919742&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/juli/FileHandler.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/juli/FileHandler.java Sat Mar  6 11:15:07 2010
@@ -26,6 +26,8 @@
 import java.io.PrintWriter;
 import java.io.UnsupportedEncodingException;
 import java.sql.Timestamp;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.logging.ErrorManager;
 import java.util.logging.Filter;
 import java.util.logging.Formatter;
@@ -96,9 +98,16 @@
      * The PrintWriter to which we are currently logging, if any.
      */
     private volatile PrintWriter writer = null;
-    
+
+
+    /**
+     * Lock used to control access to the writer.
+     */
+    protected ReadWriteLock writerLock = new ReentrantReadWriteLock();
+
+
     /**
-     * Log buffer size
+     * Log buffer size.
      */
     private int bufferSize = -1;
 
@@ -122,40 +131,52 @@
         String tsString = ts.toString().substring(0, 19);
         String tsDate = tsString.substring(0, 10);
 
+        writerLock.readLock().lock();
         // If the date has changed, switch log files
         if (!date.equals(tsDate)) {
-            synchronized (this) {
+            // Update to writeLock before we switch
+            writerLock.readLock().unlock();
+            writerLock.writeLock().lock();
+            try {
+                // Make sure another thread hasn't already done this
                 if (!date.equals(tsDate)) {
                     closeWriter();
                     date = tsDate;
                     openWriter();
                 }
+                // Down grade to read-lock. This ensures the writer remains valid
+                // until the log message is written
+                writerLock.readLock().lock();
+            } finally {
+                writerLock.writeLock().unlock();
             }
         }
 
-        String result = null;
-        try {
-            result = getFormatter().format(record);
-        } catch (Exception e) {
-            reportError(null, e, ErrorManager.FORMAT_FAILURE);
-            return;
-        }
-        
         try {
-            PrintWriter writer = this.writer;
-            if (writer!=null) {
-                writer.write(result);
-                if (bufferSize < 0) {
-                    writer.flush();
+            String result = null;
+            try {
+                result = getFormatter().format(record);
+            } catch (Exception e) {
+                reportError(null, e, ErrorManager.FORMAT_FAILURE);
+                return;
+            }
+
+            try {
+                if (writer!=null) {
+                    writer.write(result);
+                    if (bufferSize < 0) {
+                        writer.flush();
+                    }
+                } else {
+                    reportError("FileHandler is closed or not yet initialized, unable to
log ["+result+"]", null, ErrorManager.WRITE_FAILURE);
                 }
-            } else {
-                reportError("FileHandler is closed or not yet initialized, unable to log
["+result+"]", null, ErrorManager.WRITE_FAILURE);
+            } catch (Exception e) {
+                reportError(null, e, ErrorManager.WRITE_FAILURE);
+                return;
             }
-        } catch (Exception e) {
-            reportError(null, e, ErrorManager.WRITE_FAILURE);
-            return;
+        } finally {
+            writerLock.readLock().unlock();
         }
-        
     }
     
     
@@ -171,9 +192,8 @@
 
     protected void closeWriter() {
         
+        writerLock.writeLock().lock();
         try {
-            PrintWriter writer = this.writer;
-            this.writer = null;
             if (writer == null)
                 return;
             writer.write(getFormatter().getTail(this));
@@ -183,8 +203,9 @@
             date = "";
         } catch (Exception e) {
             reportError(null, e, ErrorManager.CLOSE_FAILURE);
+        } finally {
+            writerLock.writeLock().unlock();
         }
-        
     }
 
 
@@ -193,13 +214,15 @@
      */
     public void flush() {
 
+        writerLock.readLock().lock();
         try {
-            PrintWriter writer = this.writer;
-            if (writer==null)
+            if (writer == null)
                 return;
             writer.flush();
         } catch (Exception e) {
             reportError(null, e, ErrorManager.FLUSH_FAILURE);
+        } finally {
+            writerLock.readLock().unlock();
         }
         
     }
@@ -297,6 +320,7 @@
         dir.mkdirs();
 
         // Open the current log file
+        writerLock.writeLock().lock();
         try {
             String pathname = dir.getAbsolutePath() + File.separator +
                 prefix + date + suffix;
@@ -310,6 +334,8 @@
         } catch (Exception e) {
             reportError(null, e, ErrorManager.OPEN_FAILURE);
             writer = null;
+        } finally {
+            writerLock.writeLock().unlock();
         }
 
     }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=919742&r1=919741&r2=919742&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sat Mar  6 11:15:07 2010
@@ -45,6 +45,12 @@
         Close security hole in unreleased 6.0.25 by ensuring new find leaks
         functionality is protected by a security constraint. (kkolinko)
       </fix>
+      <fix>
+        <bug>48831</bug>: Improve logging shutdown behaviour. Use Catalina's
+        shutdown hook to shutdown JULI. This enables them to be shutdown in the
+        correct order. Do not shutdown global handlers several times.
+        (markt/kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">
@@ -1131,7 +1137,7 @@
       <fix>
         Correct CVE-2009-3548. When installed via the Windows installer and
         using defaults, don't create an administrative user with a blank
-        password. Additionally, the administrative user is only created of the
+        password. Additionally, the administrative user is only created if the
         manager or host-manager web applications are selected for installation.
         (markt)
       </fix>



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


Mime
View raw message