From b4264cb6369729a5825ba7527e68cb58ae6fe096 Mon Sep 17 00:00:00 2001 From: alanb Date: Sun, 19 Aug 2012 13:03:00 +0100 Subject: [PATCH] 7192275: Minimize LogManager dependencies on java.beans Summary: Reduce dependency to PropertyChangeListener and PropertyChangeEvent. Also add basic test coverage. Reviewed-by: dcubed, dsamersoff, mchung --- .../classes/java/util/logging/LogManager.java | 55 +++++-- test/java/util/logging/Listeners.java | 134 ++++++++++++++++++ test/java/util/logging/ListenersWithSM.java | 76 ++++++++++ test/java/util/logging/java.policy | 3 + 4 files changed, 259 insertions(+), 9 deletions(-) create mode 100644 test/java/util/logging/Listeners.java create mode 100644 test/java/util/logging/ListenersWithSM.java create mode 100644 test/java/util/logging/java.policy diff --git a/src/share/classes/java/util/logging/LogManager.java b/src/share/classes/java/util/logging/LogManager.java index bf8a48686..24f61b46f 100644 --- a/src/share/classes/java/util/logging/LogManager.java +++ b/src/share/classes/java/util/logging/LogManager.java @@ -32,7 +32,7 @@ import java.security.*; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; import java.beans.PropertyChangeListener; -import java.beans.PropertyChangeSupport; +import java.beans.PropertyChangeEvent; import java.net.URL; import sun.security.action.GetPropertyAction; @@ -151,10 +151,12 @@ public class LogManager { private final static Handler[] emptyHandlers = { }; private Properties props = new Properties(); - private PropertyChangeSupport changes - = new PropertyChangeSupport(LogManager.class); private final static Level defaultLevel = Level.INFO; + // The map of the registered listeners. The map value is the registration + // count to allow for cases where the same listener is registered many times. + private final Map listenerMap = new HashMap<>(); + // Table of named Loggers that maps names to Loggers. private Hashtable namedLoggers = new Hashtable<>(); // Tree of named Loggers @@ -311,11 +313,14 @@ public class LogManager { * @exception NullPointerException if the PropertyChangeListener is null. */ public void addPropertyChangeListener(PropertyChangeListener l) throws SecurityException { - if (l == null) { - throw new NullPointerException(); - } + PropertyChangeListener listener = Objects.requireNonNull(l); checkAccess(); - changes.addPropertyChangeListener(l); + synchronized (listenerMap) { + // increment the registration count if already registered + Integer value = listenerMap.get(listener); + value = (value == null) ? 1 : (value + 1); + listenerMap.put(listener, value); + } } /** @@ -334,7 +339,23 @@ public class LogManager { */ public void removePropertyChangeListener(PropertyChangeListener l) throws SecurityException { checkAccess(); - changes.removePropertyChangeListener(l); + if (l != null) { + PropertyChangeListener listener = l; + synchronized (listenerMap) { + Integer value = listenerMap.get(listener); + if (value != null) { + // remove from map if registration count is 1, otherwise + // just decrement its count + int i = value.intValue(); + if (i == 1) { + listenerMap.remove(listener); + } else { + assert i > 1; + listenerMap.put(listener, i - 1); + } + } + } + } } // Package-level method. @@ -939,7 +960,23 @@ public class LogManager { setLevelsOnExistingLoggers(); // Notify any interested parties that our properties have changed. - changes.firePropertyChange(null, null, null); + // We first take a copy of the listener map so that we aren't holding any + // locks when calling the listeners. + Map listeners = null; + synchronized (listenerMap) { + if (!listenerMap.isEmpty()) + listeners = new HashMap<>(listenerMap); + } + if (listeners != null) { + PropertyChangeEvent ev = new PropertyChangeEvent(LogManager.class, null, null, null); + for (Map.Entry entry : listeners.entrySet()) { + PropertyChangeListener listener = entry.getKey(); + int count = entry.getValue().intValue(); + for (int i = 0; i < count; i++) { + listener.propertyChange(ev); + } + } + } // Note that we need to reinitialize global handles when // they are first referenced. diff --git a/test/java/util/logging/Listeners.java b/test/java/util/logging/Listeners.java new file mode 100644 index 000000000..265fdced5 --- /dev/null +++ b/test/java/util/logging/Listeners.java @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 7192275 + * @summary Basic test of addPropertyListener/removePropertyListener methods + * @run main/othervm Listeners + */ + +import java.util.logging.LogManager; +import java.beans.PropertyChangeListener; +import java.beans.PropertyChangeEvent; + +public class Listeners { + + static void assertTrue(boolean result, String msg) { + if (!result) + throw new RuntimeException(msg); + } + + /** + * A {@code PropertyChangeListener} that counts the number of times that + * the {@code propertyChange} method is fired, and also checks that the + * event source is the expected (fixed) object. + */ + static class Listener implements PropertyChangeListener { + private final Object expectedSource; + private int fireCount; + + Listener(Object expectedSource) { + this.expectedSource = expectedSource; + } + + int fireCount() { + return fireCount; + } + + Listener reset() { + fireCount = 0; + return this; + } + + @Override + public void propertyChange(PropertyChangeEvent evt) { + assertTrue(evt.getSource() == expectedSource, "Unexpected source"); + fireCount++; + } + } + + /** + * Tests that the given listeners are invoked the expected number of + * times. + */ + static void test(Listener[] listeners, int... expected) throws Exception { + // reset counts + for (Listener listener : listeners) { + listener.reset(); + } + + // re-reading configuration causes events to be fired + LogManager.getLogManager().readConfiguration(); + + // check event listeners invoked as expected + for (int i = 0; i < expected.length; i++) { + assertTrue(listeners[i].fireCount() == expected[i], + "Unexpected event count"); + } + } + + public static void main(String[] args) throws Exception { + LogManager logman = LogManager.getLogManager(); + + Listener[] listeners = new Listener[2]; + Listener listener1 = listeners[0] = new Listener(LogManager.class); + Listener listener2 = listeners[1] = new Listener(LogManager.class); + + // add listeners + logman.addPropertyChangeListener(listener1); + test(listeners, 1, 0); + + logman.addPropertyChangeListener(listener1); + test(listeners, 2, 0); + + logman.addPropertyChangeListener(listener2); + test(listeners, 2, 1); + + // null handling to check for impact on the existing registrations + try { + logman.addPropertyChangeListener(null); + assertTrue(false, "NullPointerException expected"); + } catch (NullPointerException expected) { } + test(listeners, 2, 1); + + logman.removePropertyChangeListener(null); // no-op + test(listeners, 2, 1); + + // remove listeners + logman.removePropertyChangeListener(listener1); + test(listeners, 1, 1); + + logman.removePropertyChangeListener(listener1); + test(listeners, 0, 1); + + logman.removePropertyChangeListener(listener1); // no-op + test(listeners, 0, 1); + + logman.removePropertyChangeListener(listener2); + test(listeners, 0, 0); + + logman.removePropertyChangeListener(listener2); // no-op + test(listeners, 0, 0); + } +} diff --git a/test/java/util/logging/ListenersWithSM.java b/test/java/util/logging/ListenersWithSM.java new file mode 100644 index 000000000..41af3299b --- /dev/null +++ b/test/java/util/logging/ListenersWithSM.java @@ -0,0 +1,76 @@ +/* + * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @bug 7192275 + * @summary Basic test of addPropertyListener/removePropertyListener methods + * @run main/othervm ListenersWithSM grant + * @run main/othervm ListenersWithSM deny + */ + +import java.nio.file.Paths; +import java.util.logging.LogManager; +import java.beans.PropertyChangeListener; +import java.beans.PropertyChangeEvent; + +public class ListenersWithSM { + + public static void main(String[] args) throws Exception { + boolean granted = args[0].equals("grant"); + + // need to get reference to LogManager before setting SecurityManager + LogManager logman = LogManager.getLogManager(); + + // set policy and enable security manager + if (granted) { + String testSrc = System.getProperty("test.src"); + if (testSrc == null) + testSrc = "."; + System.setProperty("java.security.policy", + Paths.get(testSrc).resolve("java.policy").toString()); + } + System.setSecurityManager(new SecurityManager()); + + PropertyChangeListener listener = new PropertyChangeListener() { + @Override + public void propertyChange(PropertyChangeEvent evt) { + } + }; + + if (granted) { + // permission granted, no security exception expected + logman.addPropertyChangeListener(listener); + logman.removePropertyChangeListener(listener); + } else { + // denied + try { + logman.addPropertyChangeListener(listener); + throw new RuntimeException("SecurityException expected"); + } catch (SecurityException expected) { } + try { + logman.removePropertyChangeListener(listener); + throw new RuntimeException("SecurityException expected"); + } catch (SecurityException expected) { } + } + } +} diff --git a/test/java/util/logging/java.policy b/test/java/util/logging/java.policy new file mode 100644 index 000000000..e41f26073 --- /dev/null +++ b/test/java/util/logging/java.policy @@ -0,0 +1,3 @@ +grant { + permission java.util.logging.LoggingPermission "control"; +}; -- GitLab