7192275: Minimize LogManager dependencies on java.beans

Summary: Reduce dependency to PropertyChangeListener and PropertyChangeEvent. Also add basic test coverage.
Reviewed-by: dcubed, dsamersoff, mchung

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<PropertyChangeListener,Integer> listenerMap = new HashMap<>();
+
     // Table of named Loggers that maps names to Loggers.
     private Hashtable<String,LoggerWeakRef> 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<PropertyChangeListener,Integer> 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<PropertyChangeListener,Integer> 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.

test/java/util/logging/Listeners.java

+/*
+ * 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);
+    }
+}

test/java/util/logging/ListenersWithSM.java

+/*
+ * 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) { }
+        }
+    }
+}

test/java/util/logging/java.policy

+grant {
+    permission java.util.logging.LoggingPermission "control";
+};