8283092: JMX subclass permission check redundant with strong encapsulation

Reviewed-by: dfuchs, mchung

src/java.management/share/classes/java/lang/management/ManagementFactory.java

@@ -936,8 +936,7 @@ public class ManagementFactory {
                         all.add(new DefaultPlatformMBeanProvider());
                         return all;
                     }
-                }, null, new FilePermission("<<ALL FILES>>", "read"),
+                }, null, new FilePermission("<<ALL FILES>>", "read"));
-                new RuntimePermission("sun.management.spi.PlatformMBeanProvider.subclass"));
 
             // load all platform components into a map
             var map = new HashMap<String, PlatformComponent<?>>();

src/java.management/share/classes/sun/management/spi/PlatformMBeanProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2022, 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
@@ -203,15 +203,8 @@ public abstract class PlatformMBeanProvider {
 
     /**
      * Instantiates a new PlatformMBeanProvider.
-     *
-     * @throws SecurityException if the subclass (and calling code) does not
-     *    have {@code RuntimePermission("sun.management.spi.PlatformMBeanProvider.subclass")}
      */
     protected PlatformMBeanProvider () {
-        this(checkSubclassPermission());
-    }
-
-    private PlatformMBeanProvider(Void unused) {
     }
 
     /**
@@ -222,13 +215,4 @@ public abstract class PlatformMBeanProvider {
      * MBeans provided by this provider.
      */
     public abstract List<PlatformComponent<?>> getPlatformComponentList();
-
-    private static Void checkSubclassPermission() {
-        @SuppressWarnings("removal")
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(new RuntimePermission(PlatformMBeanProvider.class.getName()+".subclass"));
-        }
-        return null;
-    }
 }

src/jdk.management.agent/share/classes/jdk/internal/agent/spi/AgentProvider.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2022, 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
@@ -33,25 +33,8 @@ public abstract class AgentProvider {
 
     /**
      * Instantiates a new AgentProvider.
-     *
-     * @throws SecurityException if the subclass (and calling code) does not
-     * have
-     * {@code RuntimePermission("sun.management.spi.AgentProvider.subclass")}
      */
     protected AgentProvider() {
-        this(checkSubclassPermission());
-    }
-
-    private AgentProvider(Void unused) {
-    }
-
-    private static Void checkSubclassPermission() {
-        @SuppressWarnings("removal")
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(new RuntimePermission(AgentProvider.class.getName() + ".subclass"));
-        }
-        return null;
     }
 
     /**

test/jdk/sun/management/PlatformMBeanProviderConstructorCheck.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2022, 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
@@ -21,63 +21,79 @@
  * questions.
  */
 
-import java.security.AccessControlException;
+import java.lang.management.ManagementFactory;
-import java.security.Permission;
+import java.lang.management.RuntimeMXBean;
-import java.security.Policy;
-import java.security.ProtectionDomain;
 import java.util.List;
 
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+
+import static jdk.test.lib.Asserts.*;
+
 /*
  * @test
- * @bug     8042901
+ * @library /test/lib
- * @summary Check permission for PlatformMBeanProvider Constructor
+ * @build  jdk.test.lib.Asserts
+ * @bug     8042901 8283092
+ * @summary Check encapsulation of PlatformMBeanProvider Constructor
  * @modules java.management/sun.management.spi
- * @author  Shanliang Jiang
+ * @run main PlatformMBeanProviderConstructorCheck
- * @run main/othervm -Djava.security.manager=allow PlatformMBeanProviderConstructorCheck
  */
 public class PlatformMBeanProviderConstructorCheck {
+
+    /**
+     * jtreg invokes this test with module arguments that permit compilation
+     * of the MyProvider class, which extends PlatformMBeanProvider.
+     * First check we can invoke that class, then re-invoke ourself without
+     * those module arguments, and expect a failure calling MyProvider.
+     */
     public static void main(String[] args) throws Exception {
-        Policy origPolicy = Policy.getPolicy();
+        System.out.println("---PlatformMBeanProviderConstructorCheck:");
-        SecurityManager origSM = System.getSecurityManager();
+        boolean expectedFail = false;
-        try {
-            System.out.println("---PlatformMBeanProviderConstructorCheck starting...");
 
-            Policy.setPolicy(new MyPolicy());
+        // Recognise argument to signify we were re-invoked, and MyProvider should fail:
-            System.setSecurityManager(new SecurityManager());
+        if (args.length == 1) {
-
+            if (args[0].equals("--nomoduleargs")) {
-            System.out.println("---PlatformMBeanProviderConstructorCheck Testing without permission...");
+                expectedFail = true;
-            try {
+                verifyNoModuleArguments();
-                new MyProvider();
+            } else {
-                throw new RuntimeException("Does not get expected AccessControlException!");
+                throw new RuntimeException("unknown argument: '" + args[0] + "'");
-            } catch (AccessControlException ace) {
-                System.out.println("---PlatformMBeanProviderConstructorCheck got the expected exception: "
-                        + ace);
             }
-
+        }
-            System.out.println("---PlatformMBeanProviderConstructorCheck Testing with permission...");
+        System.out.println("---PlatformMBeanProviderConstructorCheck: invoke MyProvider with expectedFail = " + expectedFail);
-            MyPolicy.allowed = true;
+        Throwable e = null;
+        try {
             new MyProvider();
+        } catch (IllegalAccessError iae) {
+            System.out.println("---PlatformMBeanProviderConstructorCheck got exception: " + iae);
+            e = iae;
+        }
 
-            System.out.println("---PlatformMBeanProviderConstructorCheck PASSED!");
+        if (!expectedFail) {
-        } finally {
+            // This was the first invocation, should have worked OK:
-            System.setSecurityManager(origSM);
+            assertNull(e);
-            Policy.setPolicy(origPolicy);
+            System.out.println("---PlatformMBeanProviderConstructorCheck PASSED (1) (expectedFail = " + expectedFail + ")");
+
+            // Re-invoke this test to check failure:
+            System.out.println("---PlatformMBeanProviderConstructorCheck: re-invoke without --add-modules or --add-exports");
+            OutputAnalyzer output =  ProcessTools.executeTestJava("PlatformMBeanProviderConstructorCheck", "--nomoduleargs");
+            output.reportDiagnosticSummary();
+            output.shouldContain("java.lang.IllegalAccessError: superclass access check failed:");
+            output.shouldContain(" module java.management does not export sun.management.spi to ");
+            output.shouldNotContain("MyProvider constructor.");
+        } else {
+            // This was the re-invocation without module access, should fail:
+            assertNotNull(e);
+            System.out.println("---PlatformMBeanProviderConstructorCheck PASSED (2) (expectedFail = " + expectedFail + ")");
         }
     }
 
-    private static class MyPolicy extends Policy {
+    private static void verifyNoModuleArguments() {
-        private static String permName = "sun.management.spi.PlatformMBeanProvider.subclass";
+        RuntimeMXBean mxbean = ManagementFactory.getRuntimeMXBean();
-        private static boolean allowed = false;
+        for (String s : mxbean.getInputArguments()) {
-
+            if (s.startsWith("--add-modules") || s.startsWith("--add-exports")) {
-        @Override
+                System.out.println("arg: " + s);
-        public boolean implies(ProtectionDomain domain, Permission permission) {
+                throw new RuntimeException("argument list contains: " + s);
-            if (permName.equals(permission.getName())) {
-                System.out.println("---MyPolicy-implies checks permission for "
-                        +permName+" and returns "+allowed);
-
-                return allowed;
-            } else {
-                return true;
             }
         }
     }
@@ -85,6 +101,7 @@ public class PlatformMBeanProviderConstructorCheck {
     private static class MyProvider extends sun.management.spi.PlatformMBeanProvider {
         @Override
         public List<PlatformComponent<?>> getPlatformComponentList() {
+            System.out.println("MyProvider constructor.");
             return null;
         }
     }