From 6efdefd306ba84f2237bf2cff07b774bf1ccb5f2 Mon Sep 17 00:00:00 2001 From: markt Date: Wed, 3 Mar 2010 17:26:54 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48831 Address issues 1 & 2 by using a ReadWriteLock to control access to the writer. This ensures messages won't be written while the writer is null. Note there is no (easy) way to not close the handler. Address issue 3 by re-enabling the JULI shutdown hook if JULI is being used and Tomcat isn't stopped via a shutdown hook. Address issue 4 by making ClassLoaderLogManager#useShutdownHook volatile git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@918594 13f79535-47bb-0310-9956-ffa450edef68 --- java/org/apache/catalina/startup/Catalina.java | 8 ++++ java/org/apache/juli/ClassLoaderLogManager.java | 4 +- java/org/apache/juli/FileHandler.java | 54 +++++++++++++++++-------- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/java/org/apache/catalina/startup/Catalina.java b/java/org/apache/catalina/startup/Catalina.java index 8ad008bde..4bcdcb1ec 100644 --- a/java/org/apache/catalina/startup/Catalina.java +++ b/java/org/apache/catalina/startup/Catalina.java @@ -607,6 +607,14 @@ public class Catalina extends Embedded { // 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 diff --git a/java/org/apache/juli/ClassLoaderLogManager.java b/java/org/apache/juli/ClassLoaderLogManager.java index f6c49a850..993ee3fd0 100644 --- a/java/org/apache/juli/ClassLoaderLogManager.java +++ b/java/org/apache/juli/ClassLoaderLogManager.java @@ -93,9 +93,9 @@ public class ClassLoaderLogManager extends LogManager { * Determines if the shutdown hook is used to perform any necessary * clean-up such as flushing buffered handlers on JVM shutdown. Defaults to * true but may be set to false if another component ensures - * that + * that {@link #shutdown()} is called. */ - protected boolean useShutdownHook = true; + protected volatile boolean useShutdownHook = true; // ------------------------------------------------------------- Properties diff --git a/java/org/apache/juli/FileHandler.java b/java/org/apache/juli/FileHandler.java index c872f7abc..f7a6a0276 100644 --- a/java/org/apache/juli/FileHandler.java +++ b/java/org/apache/juli/FileHandler.java @@ -26,6 +26,8 @@ import java.io.OutputStreamWriter; 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 @@ public class FileHandler * 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; @@ -123,15 +132,22 @@ public class FileHandler 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) { - if (!date.equals(tsDate)) { - closeWriter(); - date = tsDate; - openWriter(); - } - } + // Update to writeLock before we switch + writerLock.readLock().unlock(); + writerLock.writeLock().lock(); + // 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(); + writerLock.writeLock().unlock(); } String result = null; @@ -139,11 +155,11 @@ public class FileHandler result = getFormatter().format(record); } catch (Exception e) { reportError(null, e, ErrorManager.FORMAT_FAILURE); + writerLock.readLock().unlock(); return; } try { - PrintWriter writer = this.writer; if (writer!=null) { writer.write(result); if (bufferSize < 0) { @@ -155,8 +171,9 @@ public class FileHandler } catch (Exception e) { reportError(null, e, ErrorManager.WRITE_FAILURE); return; + } finally { + writerLock.readLock().unlock(); } - } @@ -174,8 +191,7 @@ public class FileHandler protected void closeWriter() { try { - PrintWriter writer = this.writer; - this.writer = null; + writerLock.writeLock().lock(); if (writer == null) return; writer.write(getFormatter().getTail(this)); @@ -185,8 +201,9 @@ public class FileHandler date = ""; } catch (Exception e) { reportError(null, e, ErrorManager.CLOSE_FAILURE); + } finally { + writerLock.writeLock().unlock(); } - } @@ -197,12 +214,14 @@ public class FileHandler public void flush() { try { - PrintWriter writer = this.writer; - if (writer==null) + writerLock.readLock().lock(); + if (writer == null) return; writer.flush(); } catch (Exception e) { reportError(null, e, ErrorManager.FLUSH_FAILURE); + } finally { + writerLock.readLock().unlock(); } } @@ -306,6 +325,7 @@ public class FileHandler String encoding = getEncoding(); FileOutputStream fos = new FileOutputStream(pathname, true); OutputStream os = bufferSize>0?new BufferedOutputStream(fos,bufferSize):fos; + writerLock.writeLock().lock(); writer = new PrintWriter( (encoding != null) ? new OutputStreamWriter(os, encoding) : new OutputStreamWriter(os), false); @@ -313,6 +333,8 @@ public class FileHandler } catch (Exception e) { reportError(null, e, ErrorManager.OPEN_FAILURE); writer = null; + } finally { + writerLock.writeLock().unlock(); } } -- 2.11.0