diff options
9 files changed, 221 insertions, 154 deletions
@@ -1,3 +1,30 @@ +2013-12-20 Jiri Vanek <[email protected]> + + singletons logic, logs and test cleanup/fixes + * netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java: + config singleton made properly synchronized via Holder pattern + (DeploymentConfigurationHolder). + * netx/net/sourceforge/jnlp/util/logging/JavaConsole.java: same, + (JavaConsoleHolder). Console gui initialized on show, not on creation. + Removed look and feel. (addMessage) gui update adapted. + * netx/net/sourceforge/jnlp/util/logging/LogConfig.java: made private, + singleton handled via LogConfigHolder. + * netx/net/sourceforge/jnlp/util/logging/OutputController.java: mentioned + issue with (getConfiguration), removed obsoleted (MessageWithLevel), + (messageQue) retyped to <MessageWithHeader>, (consume) adapted. + (consumerThread) made global variable, ist start moved to (startConsumer) + which is called after initialisation of config singleton. Logs queing + moved to (log) of (MessageWithHeader) signature. (FileLogHolder) and + (SystemLogHolder) created for holder pattern synchronization. + * netx/net/sourceforge/jnlp/util/logging/headers/Header.java: constructor + and (getCaller) adaptation. + * netx/net/sourceforge/jnlp/util/logging/headers/PluginHeader.java: fixed + (toString) for preinit messages. + * tests/netx/unit/net/sourceforge/jnlp/util/logging/JavaConsoleTest.java: + removed erroneous stdout. + * tests/test-extensions/net/sourceforge/jnlp/util/logging/NoStdOutErrTest.java: + is no longer throwing exceptions (was causing errors in junit) and synchronized. + 2013-12-17 Jiri Vanek <[email protected]> JNLPRuntime.config changed to proper singleton. diff --git a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java index 3ec3f91..ac510ba 100644 --- a/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java +++ b/netx/net/sourceforge/jnlp/runtime/JNLPRuntime.java @@ -90,8 +90,6 @@ public class JNLPRuntime { /** the localized resource strings */ private static ResourceBundle resources; - private static DeploymentConfiguration config; - /** the security manager */ private static JNLPSecurityManager security; @@ -351,25 +349,39 @@ public class JNLPRuntime { /** - * Gets the Configuration associated with this runtime - * @return a {@link DeploymentConfiguration} object that can be queried to - * find relevant configuration settings + * see https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + * for cases how not to do lazy initialization + * and https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + * for ITW approach */ - public synchronized static DeploymentConfiguration getConfiguration() { - if (config == null){ - config = new DeploymentConfiguration(); - try{ + private static class DeploymentConfigurationHolder { + + private static final DeploymentConfiguration INSTANCE = initConfiguration(); + + private static DeploymentConfiguration initConfiguration() { + DeploymentConfiguration config = new DeploymentConfiguration(); + try { config.load(); config.copyTo(System.getProperties()); - }catch(ConfigurationException ex){ - OutputController.getLogger().log(ex); - //mark first occurence of exception so we can react later - if (config.getLoadingException() == null){ - config.setLoadingException(ex); - } + } catch (ConfigurationException ex) { + OutputController.getLogger().log(OutputController.Level.MESSAGE_ALL, getMessage("RConfigurationError")); + //mark this exceptionas we can die on it later + config.setLoadingException(ex); + } finally { + OutputController.getLogger().startConsumer(); } + return config; } - return config; + } + + /** + * Gets the Configuration associated with this runtime + * + * @return a {@link DeploymentConfiguration} object that can be queried to + * find relevant configuration settings + */ + public static DeploymentConfiguration getConfiguration() { + return DeploymentConfigurationHolder.INSTANCE; } /** diff --git a/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java b/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java index 2ee601e..3b7d438 100644 --- a/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java +++ b/netx/net/sourceforge/jnlp/util/logging/JavaConsole.java @@ -62,15 +62,12 @@ import javax.swing.JScrollPane; import javax.swing.JSplitPane; import javax.swing.JTextArea; import javax.swing.SwingUtilities; -import javax.swing.UIManager; import javax.swing.border.EmptyBorder; import javax.swing.border.TitledBorder; import net.sourceforge.jnlp.config.DeploymentConfiguration; import net.sourceforge.jnlp.runtime.JNLPRuntime; import net.sourceforge.jnlp.util.ImageResources; import net.sourceforge.jnlp.util.logging.headers.Header; -import net.sourceforge.jnlp.util.logging.headers.JavaMessage; -import net.sourceforge.jnlp.util.logging.headers.MessageWithHeader; import net.sourceforge.jnlp.util.logging.headers.PluginMessage; /** @@ -86,11 +83,14 @@ public class JavaConsole { private static JavaConsole console; private static Dimension lastSize; + private static class JavaConsoleHolder { + + //https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + //https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + private static final JavaConsole INSTANCE = new JavaConsole(); + } public static JavaConsole getConsole() { - if (console == null) { - console = new JavaConsole(); - } - return console; + return JavaConsoleHolder.INSTANCE; } public static boolean isEnabled() { @@ -119,15 +119,22 @@ public class JavaConsole { private JDialog consoleWindow; private JTextArea stdErrText; private JTextArea stdOutText; - private JPanel contentPanel = new JPanel(); + private JPanel contentPanel; private ClassLoaderInfoProvider classLoaderInfoProvider; + private boolean initialized = false; + + private String stdErrTextSrc = ""; + private String stdOutTextSrc = ""; public JavaConsole() { - initialize(); + } private void initializeWindow() { + if (!initialized){ + initialize(); + } initializeWindow(lastSize, contentPanel); } @@ -160,13 +167,6 @@ public class JavaConsole { */ private void initialize() { - try { - UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName()); - } catch (Exception e) { - OutputController.getLogger().log(OutputController.Level.ERROR_ALL, e); - } - - contentPanel = new JPanel(); contentPanel.setLayout(new GridBagLayout()); @@ -296,6 +296,7 @@ public class JavaConsole { splitPane.setDividerLocation(0.5); splitPane.setResizeWeight(0.5); + initialized = true; } public void showConsole() { @@ -416,22 +417,21 @@ public class JavaConsole { } - void addMessage(Header header, String message) { - if (!LogConfig.getLogConfig().isEnableHeaders()){ + synchronized void addMessage(Header header, String message) { + String headerString = ""; + if (LogConfig.getLogConfig().isEnableHeaders()){ + headerString = header.toString(); + } if (header.level.isError()){ - stdErrText.setText(stdErrText.getText() + message + "\n"); + stdErrTextSrc += headerString + message +"\n"; } if (header.level.isOutput()){ - stdOutText.setText(stdOutText.getText() + message + "\n"); + stdOutTextSrc += headerString + message + "\n"; } - } else { - if (header.level.isError()){ - stdErrText.setText(stdErrText.getText( )+ header.toString() + message +"\n"); + if (initialized){ + stdErrText.setText(stdErrTextSrc); + stdOutText.setText(stdOutTextSrc); } - if (header.level.isOutput()){ - stdOutText.setText(stdOutText.getText() + header.toString() + message + "\n"); - } - } } /** @@ -440,7 +440,7 @@ public class JavaConsole { */ private void processPluginMessage(String s) { PluginMessage pm = new PluginMessage(s); - addMessage(pm.getHeader(), pm.getMessage()); + OutputController.getLogger().log(pm); } diff --git a/netx/net/sourceforge/jnlp/util/logging/LogConfig.java b/netx/net/sourceforge/jnlp/util/logging/LogConfig.java index c02043c..18bcd74 100644 --- a/netx/net/sourceforge/jnlp/util/logging/LogConfig.java +++ b/netx/net/sourceforge/jnlp/util/logging/LogConfig.java @@ -37,8 +37,6 @@ package net.sourceforge.jnlp.util.logging; import java.io.File; -import javax.naming.ConfigurationException; - import net.sourceforge.jnlp.config.DeploymentConfiguration; import net.sourceforge.jnlp.runtime.JNLPRuntime; @@ -56,9 +54,7 @@ public class LogConfig { private boolean logToStreams; private boolean logToSysLog; - private static LogConfig logConfig; - - public LogConfig() { + private LogConfig() { DeploymentConfiguration config = JNLPRuntime.getConfiguration(); // Check whether logging and tracing is enabled. enableLogging = Boolean.parseBoolean(config.getProperty(DeploymentConfiguration.KEY_ENABLE_LOGGING)); @@ -81,18 +77,20 @@ public class LogConfig { } } + private static class LogConfigHolder { + + //https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + //https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + private static volatile LogConfig INSTANCE = new LogConfig(); + } + public static LogConfig getLogConfig() { - if (logConfig == null) { - logConfig = new LogConfig(); - } - return logConfig; + return LogConfigHolder.INSTANCE; } /** For testing only: throw away the previous config */ - static void resetLogConfig() { - if (logConfig != null) { - logConfig = new LogConfig(); - } + static synchronized void resetLogConfig() { + LogConfigHolder.INSTANCE = new LogConfig(); } public String getIcedteaLogDir() { diff --git a/netx/net/sourceforge/jnlp/util/logging/OutputController.java b/netx/net/sourceforge/jnlp/util/logging/OutputController.java index d84950a..996eebb 100644 --- a/netx/net/sourceforge/jnlp/util/logging/OutputController.java +++ b/netx/net/sourceforge/jnlp/util/logging/OutputController.java @@ -38,11 +38,19 @@ package net.sourceforge.jnlp.util.logging; import java.io.PrintStream; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Date; import java.util.LinkedList; import java.util.List; import net.sourceforge.jnlp.runtime.JNLPRuntime; +import net.sourceforge.jnlp.util.logging.headers.Header; +import net.sourceforge.jnlp.util.logging.headers.JavaMessage; +import net.sourceforge.jnlp.util.logging.headers.MessageWithHeader; + +/** + * + * OutputController class (thread) must NOT call JNLPRuntime.getConfiguraion() + * + */ public class OutputController { public static enum Level { @@ -88,30 +96,15 @@ public class OutputController { } } - private static final class MessageWithLevel { - - public final String message; - public final Level level; - public final StackTraceElement[] stack = Thread.currentThread().getStackTrace(); - public final Thread thread = Thread.currentThread(); - public final Date loggedAt = new Date(); - - public MessageWithLevel(String message, Level level) { - this.message = message; - this.level = level; - } - } /* * singleton instance */ - private static OutputController logger; private static final String NULL_OBJECT = "Trying to log null object"; - private FileLog fileLog; private PrintStreamLogger outLog; private PrintStreamLogger errLog; - private SingleStreamLogger sysLog; - private List<MessageWithLevel> messageQue = new LinkedList<MessageWithLevel>(); + private List<MessageWithHeader> messageQue = new LinkedList<MessageWithHeader>(); private MessageQueConsumer messageQueConsumer = new MessageQueConsumer(); + Thread consumerThread; //bounded to instance private class MessageQueConsumer implements Runnable { @@ -149,33 +142,32 @@ public class OutputController { } private void consume() { - MessageWithLevel s = messageQue.get(0); + MessageWithHeader s = messageQue.get(0); messageQue.remove(0); - net.sourceforge.jnlp.util.logging.headers.Header header = new net.sourceforge.jnlp.util.logging.headers.Header(s.level, s.stack, s.thread, s.loggedAt, false); //filtering is done in console during runtime if (LogConfig.getLogConfig().isLogToConsole()) { - JavaConsole.getConsole().addMessage(header, s.message); + JavaConsole.getConsole().addMessage(s.getHeader(), s.getMessage()); } - if (!JNLPRuntime.isDebug() && (s.level == Level.MESSAGE_DEBUG - || s.level == Level.WARNING_DEBUG - || s.level == Level.ERROR_DEBUG)) { + if (!JNLPRuntime.isDebug() && (s.getHeader().level == Level.MESSAGE_DEBUG + || s.getHeader().level == Level.WARNING_DEBUG + || s.getHeader().level == Level.ERROR_DEBUG)) { //filter out debug messages //must be here to prevent deadlock, casued by exception form jnlpruntime, loggers or configs themselves return; } - String message = s.message; + String message = s.getMessage(); if (LogConfig.getLogConfig().isEnableHeaders()) { if (message.contains("\n")) { - message = header.toString() + "\n" + message; + message = s.getHeader().toString() + "\n" + message; } else { - message = header.toString() + " " + message; + message = s.getHeader().toString() + " " + message; } } if (LogConfig.getLogConfig().isLogToStreams()) { - if (s.level.isOutput()) { + if (s.getHeader().level.isOutput()) { outLog.log(message); } - if (s.level.isError()) { + if (s.getHeader().level.isError()) { errLog.log(message); } } @@ -191,17 +183,22 @@ public class OutputController { private OutputController() { this(System.out, System.err); } + + + private static class OutputControllerHolder { + + //https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + //https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + private static final OutputController INSTANCE = new OutputController(); + } /** * This should be the only legal way to get logger for ITW * * @return logging singleton */ - synchronized public static OutputController getLogger() { - if (logger == null) { - logger = new OutputController(); - } - return logger; + public static OutputController getLogger() { + return OutputControllerHolder.INSTANCE; } /** @@ -215,22 +212,24 @@ public class OutputController { outLog = new PrintStreamLogger(out); errLog = new PrintStreamLogger(err); //itw logger have to be fully initialised before start - Thread t = new Thread(messageQueConsumer, "Output controller consumer daemon"); - t.setDaemon(true); - t.start(); - //some messages were probably posted before start of consumer - synchronized (this){ - this.notifyAll(); - } + consumerThread = new Thread(messageQueConsumer, "Output controller consumer daemon"); + consumerThread.setDaemon(true); + //is started in JNLPRuntime.getConfig() after config is laoded Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() { @Override public void run() { - while (!messageQue.isEmpty()) { - consume(); - } + flush(); } })); } + + public void startConsumer() { + consumerThread.start(); + //some messages were probably posted before start of consumer + synchronized (this) { + this.notifyAll(); + } + } /** * @@ -302,33 +301,52 @@ public class OutputController { log(Level.ERROR_DEBUG, (Object) s); } - private synchronized void log(Level level, Object o) { + private void log(Level level, Object o) { + String s =""; if (o == null) { - messageQue.add(new MessageWithLevel(NULL_OBJECT, level)); + s = NULL_OBJECT; } else if (o instanceof Throwable) { - messageQue.add(new MessageWithLevel(exceptionToString((Throwable) o), level)); + s = exceptionToString((Throwable) o); } else { - messageQue.add(new MessageWithLevel(o.toString(), level)); + s=o.toString(); } + log(new JavaMessage(new Header(level, false), s)); + } + + synchronized void log(MessageWithHeader l){ + messageQue.add(l); this.notifyAll(); } + + + private static class FileLogHolder { + + //https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + //https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + private static volatile FileLog INSTANCE = new FileLog(); + } private FileLog getFileLog() { - if (fileLog == null) { - fileLog = new FileLog(); - } - return fileLog; + return FileLogHolder.INSTANCE; } - private SingleStreamLogger getSystemLog() { - if (sysLog == null) { + private static class SystemLogHolder { + + //https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java + //https://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom + private static volatile SingleStreamLogger INSTANCE = initSystemLogger(); + + private static SingleStreamLogger initSystemLogger() { if (JNLPRuntime.isWindows()) { - sysLog = new WinSystemLog(); + return new WinSystemLog(); } else { - sysLog = new UnixSystemLog(); + return new UnixSystemLog(); } } - return sysLog; + } + + private SingleStreamLogger getSystemLog() { + return SystemLogHolder.INSTANCE; } public void printErrorLn(String e) { @@ -368,7 +386,7 @@ public class OutputController { } void setFileLog(FileLog fileLog) { - this.fileLog = fileLog; + FileLogHolder.INSTANCE = fileLog; } void setOutLog(PrintStreamLogger outLog) { @@ -376,7 +394,7 @@ public class OutputController { } void setSysLog(SingleStreamLogger sysLog) { - this.sysLog = sysLog; + SystemLogHolder.INSTANCE = sysLog; } diff --git a/netx/net/sourceforge/jnlp/util/logging/headers/Header.java b/netx/net/sourceforge/jnlp/util/logging/headers/Header.java index a87a14f..bb09f89 100644 --- a/netx/net/sourceforge/jnlp/util/logging/headers/Header.java +++ b/netx/net/sourceforge/jnlp/util/logging/headers/Header.java @@ -42,8 +42,9 @@ import net.sourceforge.jnlp.util.logging.OutputController; import net.sourceforge.jnlp.util.logging.OutputController.Level; public class Header { - - public String user; + public static String default_user = System.getProperty("user.name"); + + public String user = default_user; public boolean application; public Level level; public Date date = new Date(); @@ -56,12 +57,15 @@ public class Header { public Header() { } + public Header(Level level, boolean isC) { + this(level, Thread.currentThread().getStackTrace(), Thread.currentThread(), isC); + } + public Header(Level level, StackTraceElement[] stack, Thread thread, boolean isC) { this(level, stack, thread, new Date(), isC); } public Header(Level level, StackTraceElement[] stack, Thread thread, Date d, boolean isC) { - this.user = System.getProperty("user.name"); this.application = JNLPRuntime.isWebstartApplication(); this.level = level; this.date = d; @@ -146,7 +150,8 @@ public class Header { result = stack[i];//at least moving up if (stack[i].getClassName().contains(OutputController.class.getName()) || //PluginDebug.class.getName() not avaiable during netx make - stack[i].getClassName().contains("sun.applet.PluginDebug")) { + stack[i].getClassName().contains("sun.applet.PluginDebug") + || stack[i].getClassName().contains(Header.class.getName())) { continue; } else { break; diff --git a/netx/net/sourceforge/jnlp/util/logging/headers/PluginHeader.java b/netx/net/sourceforge/jnlp/util/logging/headers/PluginHeader.java index 31ed19d..834085f 100644 --- a/netx/net/sourceforge/jnlp/util/logging/headers/PluginHeader.java +++ b/netx/net/sourceforge/jnlp/util/logging/headers/PluginHeader.java @@ -52,15 +52,12 @@ public class PluginHeader extends Header { static final Pattern whiteSpaces = Pattern.compile("\\s+"); static final Pattern threadsPattern = Pattern.compile("\\s+|,\\s*|:"); + @Override public String toString() { - if (preinit) { - return "!" + super.toString(); - } else { - return super.toString(); - } + return toString(true, true, true, true, true, true, true); } - + @Override public String toString(boolean userb, boolean originb, boolean levelb, boolean dateb, boolean callerb, boolean thread1b, boolean thread2b) { if (preinit) { diff --git a/tests/netx/unit/net/sourceforge/jnlp/util/logging/JavaConsoleTest.java b/tests/netx/unit/net/sourceforge/jnlp/util/logging/JavaConsoleTest.java index 8b36bac..b4ab4c8 100644 --- a/tests/netx/unit/net/sourceforge/jnlp/util/logging/JavaConsoleTest.java +++ b/tests/netx/unit/net/sourceforge/jnlp/util/logging/JavaConsoleTest.java @@ -21,7 +21,6 @@ public class JavaConsoleTest { @Test public void CreatePluginHeaderTestOK() throws Exception{ PluginMessage p1 = new PluginMessage(s1); - System.out.println(p1.header + p1.restOfMessage); PluginMessage p3 = new PluginMessage(s3); Assert.assertFalse(p1.wasError); Assert.assertFalse(p3.wasError); diff --git a/tests/test-extensions/net/sourceforge/jnlp/util/logging/NoStdOutErrTest.java b/tests/test-extensions/net/sourceforge/jnlp/util/logging/NoStdOutErrTest.java index ee9e977..456c9d2 100644 --- a/tests/test-extensions/net/sourceforge/jnlp/util/logging/NoStdOutErrTest.java +++ b/tests/test-extensions/net/sourceforge/jnlp/util/logging/NoStdOutErrTest.java @@ -34,11 +34,11 @@ this exception to your version of the library, but you are not obligated to do so. If you do not wish to do so, delete this exception statement from your version. */ + package net.sourceforge.jnlp.util.logging; -import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import net.sourceforge.jnlp.ServerAccess; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -54,36 +54,47 @@ import org.junit.BeforeClass; * by junit classlaoder is visible from itw, but not vice verse. */ public class NoStdOutErrTest { - + private static boolean origialStds; - private static final String setLogToStreams = "setLogToStreams"; - - @BeforeClass - public static void disableStds() throws Exception { - //init logger and log and flush message - //it is crucial for junit to grip it - OutputController.getLogger().log("initialising"); - //one more times: if TESTED class is the first which creates instance of logger - //then when junit can not access this class, and creates its own for its purposes - //when junit creates this class, then also TESTED class have access to it and so it behaves as expected - OutputController.getLogger().flush(); - origialStds = LogConfig.getLogConfig().isLogToStreams(); - invokeSetLogToStreams(false); + /* + * "printed" exceptions are otherwise consumed via junit if thrown :-/ + */ + + @BeforeClass + public static synchronized void disableStds() { + try { + //init logger and log and flush message + //it is crucial for junit to grip it + OutputController.getLogger().log("initialising"); + //one more times: if TESTED class is the first which creates instance of logger + //then when junit can not access this class, and creates its own for its purposes + //when junit creates this class, then also TESTED class have access to it and so it behaves as expected + OutputController.getLogger().flush(); + origialStds = LogConfig.getLogConfig().isLogToStreams(); + invokeSetLogToStreams(false); + } catch (Exception ex) { + ServerAccess.logException(ex); + } } - + @AfterClass - public static void restoreStds() throws Exception { - OutputController.getLogger().flush(); - invokeSetLogToStreams(origialStds); + public static synchronized void restoreStds() { + try { + OutputController.getLogger().flush(); + invokeSetLogToStreams(origialStds); + } catch (Exception ex) { + ServerAccess.logException(ex); + } } - private static void invokeSetLogToStreams(boolean state) throws IllegalAccessException, SecurityException, NoSuchMethodException, InvocationTargetException, IllegalArgumentException { - Method lcs = LogConfig.class.getDeclaredMethod(setLogToStreams, boolean.class); - lcs.setAccessible(true); - lcs.invoke(LogConfig.getLogConfig(), state); + private static synchronized void invokeSetLogToStreams(boolean state) { + try { + Method lcs = LogConfig.class.getDeclaredMethod(setLogToStreams, boolean.class); + lcs.setAccessible(true); + lcs.invoke(LogConfig.getLogConfig(), state); + } catch (Exception ex) { + ServerAccess.logException(ex); + } } - - - } |