Mercurial > hg > openjdk > aarch64-port > jdk
changeset 8030:ac6e99af2056
6823527: java.util.logging.Handler has thread safety issues
Reviewed-by: dholmes, mchung
| author | dfuchs |
|---|---|
| date | Wed, 04 Sep 2013 15:32:59 +0200 |
| parents | 336784cd60c3 |
| children | b3d6953b9829 |
| files | src/share/classes/java/util/logging/ConsoleHandler.java src/share/classes/java/util/logging/FileHandler.java src/share/classes/java/util/logging/Handler.java src/share/classes/java/util/logging/MemoryHandler.java src/share/classes/java/util/logging/SocketHandler.java src/share/classes/java/util/logging/StreamHandler.java |
| diffstat | 6 files changed, 50 insertions(+), 26 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/java/util/logging/ConsoleHandler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/ConsoleHandler.java Wed Sep 04 15:32:59 2013 +0200 @@ -26,9 +26,6 @@ package java.util.logging; -import java.io.*; -import java.net.*; - /** * This <tt>Handler</tt> publishes log records to <tt>System.err</tt>. * By default the <tt>SimpleFormatter</tt> is used to generate brief summaries. @@ -114,6 +111,7 @@ * @param record description of the log event. A null record is * silently ignored and is not published */ + @Override public void publish(LogRecord record) { super.publish(record); flush(); @@ -124,6 +122,7 @@ * to close the output stream. That is, we do <b>not</b> * close <tt>System.err</tt>. */ + @Override public void close() { flush(); }
--- a/src/share/classes/java/util/logging/FileHandler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/FileHandler.java Wed Sep 04 15:32:59 2013 +0200 @@ -149,7 +149,7 @@ private FileChannel lockFileChannel; private File files[]; private static final int MAX_LOCKS = 100; - private static java.util.HashMap<String, String> locks = new java.util.HashMap<>(); + private static final java.util.HashMap<String, String> locks = new java.util.HashMap<>(); /** * A metered stream is a subclass of OutputStream that @@ -157,7 +157,7 @@ * (b) keeps track of how many bytes have been written */ private class MeteredStream extends OutputStream { - OutputStream out; + final OutputStream out; int written; MeteredStream(OutputStream out, int written) { @@ -165,25 +165,30 @@ this.written = written; } + @Override public void write(int b) throws IOException { out.write(b); written++; } + @Override public void write(byte buff[]) throws IOException { out.write(buff); written += buff.length; } + @Override public void write(byte buff[], int off, int len) throws IOException { out.write(buff,off,len); written += len; } + @Override public void flush() throws IOException { out.flush(); } + @Override public void close() throws IOException { out.close(); } @@ -607,6 +612,7 @@ * @param record description of the log event. A null record is * silently ignored and is not published */ + @Override public synchronized void publish(LogRecord record) { if (!isLoggable(record)) { return; @@ -620,6 +626,7 @@ // currently being called from untrusted code. // So it is safe to raise privilege here. AccessController.doPrivileged(new PrivilegedAction<Object>() { + @Override public Object run() { rotate(); return null; @@ -634,6 +641,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ + @Override public synchronized void close() throws SecurityException { super.close(); // Unlock any lock file. @@ -656,6 +664,7 @@ private static class InitializationErrorManager extends ErrorManager { Exception lastException; + @Override public void error(String msg, Exception ex, int code) { lastException = ex; }
--- a/src/share/classes/java/util/logging/Handler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/Handler.java Wed Sep 04 15:32:59 2013 +0200 @@ -47,12 +47,20 @@ public abstract class Handler { private static final int offValue = Level.OFF.intValue(); - private LogManager manager = LogManager.getLogManager(); - private Filter filter; - private Formatter formatter; - private Level logLevel = Level.ALL; - private ErrorManager errorManager = new ErrorManager(); - private String encoding; + private final LogManager manager = LogManager.getLogManager(); + + // We're using volatile here to avoid synchronizing getters, which + // would prevent other threads from calling isLoggable() + // while publish() is executing. + // On the other hand, setters will be synchronized to exclude concurrent + // execution with more complex methods, such as StreamHandler.publish(). + // We wouldn't want 'level' to be changed by another thread in the middle + // of the execution of a 'publish' call. + private volatile Filter filter; + private volatile Formatter formatter; + private volatile Level logLevel = Level.ALL; + private volatile ErrorManager errorManager = new ErrorManager(); + private volatile String encoding; // Package private support for security checking. When sealed // is true, we access check updates to the class. @@ -110,7 +118,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ - public void setFormatter(Formatter newFormatter) throws SecurityException { + public synchronized void setFormatter(Formatter newFormatter) throws SecurityException { checkPermission(); // Check for a null pointer: newFormatter.getClass(); @@ -138,7 +146,7 @@ * @exception UnsupportedEncodingException if the named encoding is * not supported. */ - public void setEncoding(String encoding) + public synchronized void setEncoding(String encoding) throws SecurityException, java.io.UnsupportedEncodingException { checkPermission(); if (encoding != null) { @@ -174,7 +182,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ - public void setFilter(Filter newFilter) throws SecurityException { + public synchronized void setFilter(Filter newFilter) throws SecurityException { checkPermission(); filter = newFilter; } @@ -198,7 +206,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ - public void setErrorManager(ErrorManager em) { + public synchronized void setErrorManager(ErrorManager em) { checkPermission(); if (em == null) { throw new NullPointerException(); @@ -264,7 +272,7 @@ * than this level will be discarded. * @return the level of messages being logged. */ - public synchronized Level getLevel() { + public Level getLevel() { return logLevel; } @@ -282,11 +290,11 @@ * */ public boolean isLoggable(LogRecord record) { - int levelValue = getLevel().intValue(); + final int levelValue = getLevel().intValue(); if (record.getLevel().intValue() < levelValue || levelValue == offValue) { return false; } - Filter filter = getFilter(); + final Filter filter = getFilter(); if (filter == null) { return true; }
--- a/src/share/classes/java/util/logging/MemoryHandler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/MemoryHandler.java Wed Sep 04 15:32:59 2013 +0200 @@ -88,7 +88,7 @@ public class MemoryHandler extends Handler { private final static int DEFAULT_SIZE = 1000; - private Level pushLevel; + private volatile Level pushLevel; private int size; private Handler target; private LogRecord buffer[]; @@ -188,6 +188,7 @@ * @param record description of the log event. A null record is * silently ignored and is not published */ + @Override public synchronized void publish(LogRecord record) { if (!isLoggable(record)) { return; @@ -227,6 +228,7 @@ * Note that the current contents of the <tt>MemoryHandler</tt> * buffer are <b>not</b> written out. That requires a "push". */ + @Override public void flush() { target.flush(); } @@ -238,6 +240,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ + @Override public void close() throws SecurityException { target.close(); setLevel(Level.OFF); @@ -252,11 +255,10 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ - public void setPushLevel(Level newLevel) throws SecurityException { + public synchronized void setPushLevel(Level newLevel) throws SecurityException { if (newLevel == null) { throw new NullPointerException(); } - LogManager manager = LogManager.getLogManager(); checkPermission(); pushLevel = newLevel; } @@ -266,7 +268,7 @@ * * @return the value of the <tt>pushLevel</tt> */ - public synchronized Level getPushLevel() { + public Level getPushLevel() { return pushLevel; } @@ -283,6 +285,7 @@ * @return true if the <tt>LogRecord</tt> would be logged. * */ + @Override public boolean isLoggable(LogRecord record) { return super.isLoggable(record); }
--- a/src/share/classes/java/util/logging/SocketHandler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/SocketHandler.java Wed Sep 04 15:32:59 2013 +0200 @@ -82,7 +82,6 @@ private Socket sock; private String host; private int port; - private String portProperty; // Private method to configure a SocketHandler from LogManager // properties and/or default values as specified in the class @@ -177,6 +176,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have <tt>LoggingPermission("control")</tt>. */ + @Override public synchronized void close() throws SecurityException { super.close(); if (sock != null) { @@ -195,6 +195,7 @@ * @param record description of the log event. A null record is * silently ignored and is not published */ + @Override public synchronized void publish(LogRecord record) { if (!isLoggable(record)) { return;
--- a/src/share/classes/java/util/logging/StreamHandler.java Wed Sep 04 11:40:23 2013 +0100 +++ b/src/share/classes/java/util/logging/StreamHandler.java Wed Sep 04 15:32:59 2013 +0200 @@ -73,10 +73,9 @@ */ public class StreamHandler extends Handler { - private LogManager manager = LogManager.getLogManager(); private OutputStream output; private boolean doneHeader; - private Writer writer; + private volatile Writer writer; // Private method to configure a StreamHandler from LogManager // properties and/or default values as specified in the class @@ -169,7 +168,8 @@ * @exception UnsupportedEncodingException if the named encoding is * not supported. */ - public void setEncoding(String encoding) + @Override + public synchronized void setEncoding(String encoding) throws SecurityException, java.io.UnsupportedEncodingException { super.setEncoding(encoding); if (output == null) { @@ -201,6 +201,7 @@ * @param record description of the log event. A null record is * silently ignored and is not published */ + @Override public synchronized void publish(LogRecord record) { if (!isLoggable(record)) { return; @@ -240,6 +241,7 @@ * @return true if the <tt>LogRecord</tt> would be logged. * */ + @Override public boolean isLoggable(LogRecord record) { if (writer == null || record == null) { return false; @@ -250,6 +252,7 @@ /** * Flush any buffered messages. */ + @Override public synchronized void flush() { if (writer != null) { try { @@ -294,6 +297,7 @@ * @exception SecurityException if a security manager exists and if * the caller does not have LoggingPermission("control"). */ + @Override public synchronized void close() throws SecurityException { flushAndClose(); }