RFR: 8302791: Add specific ClassLoader object to Proxy IllegalArgumentException message [v4]

[Mandy Chung]

On Thu, 2 Mar 2023 14:18:13 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/reflect/Proxy.java line 889:
>> 
>>> 887:                                                     : Objects.toIdentityString(ld);
>>> 888:                 throw new IllegalArgumentException(c.getName() +
>>> 889:                         " referenced from a method is not visible from class loader: " + nid);
>> 
>> Thanks for the update, this looks much better. A passing thought is that you might want to use "/" instead of "@" as the separator so that someone looking at the exception message will be able to distinguish a class name from a class loader name. Also you might want to re-format L886 to avoid the really long line as it's a bit inconsistent with the code in this source file.
>
>> Thanks @AlanBateman. Regarding the separator, Have seen this being used in ClassLoader.java for nameAndId method. Hence used the same for consistency. I've reformatted L886 now, Please kindly check.
> 
> Thanks for checking, the updated version looks good.

I suggest to add a `JavaLangAccess::getLoaderNameID(ClassLoader loader)` so that Proxy (and other classes) can use the same representation.

The patch will look like this:


diff --git a/src/java.base/share/classes/java/lang/ClassLoader.java b/src/java.base/share/classes/java/lang/ClassLoader.java
index 2bc9a22700a..f412ecfa3ae 100644
--- a/src/java.base/share/classes/java/lang/ClassLoader.java
+++ b/src/java.base/share/classes/java/lang/ClassLoader.java
@@ -407,6 +407,10 @@ public abstract class ClassLoader {
         return nid;
     }
 
+    String nameAndId() {
+        return nameAndId;
+    }
+
     /**
      * Creates a new class loader of the specified name and using the
      * specified parent class loader for delegation.
diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java
index 501ed47fcad..c138ea9fef5 100644
--- a/src/java.base/share/classes/java/lang/System.java
+++ b/src/java.base/share/classes/java/lang/System.java
@@ -2663,6 +2663,10 @@ public final class System {
                                                       Continuation continuation) {
                 return StackWalker.newInstance(options, null, contScope, continuation);
             }
+
+            public String getLoaderNameID(ClassLoader loader) {
+                return loader.nameAndId();
+            }
         });
     }
 }
diff --git a/src/java.base/share/classes/java/lang/reflect/Proxy.java b/src/java.base/share/classes/java/lang/reflect/Proxy.java
index 6064dcf5b6b..a2a9a03e6c4 100644
--- a/src/java.base/share/classes/java/lang/reflect/Proxy.java
+++ b/src/java.base/share/classes/java/lang/reflect/Proxy.java
@@ -878,7 +878,7 @@ public class Proxy implements java.io.Serializable {
             }
             if (type != c) {
                 throw new IllegalArgumentException(c.getName() +
-                        " referenced from a method is not visible from class loader");
+                        " referenced from a method is not visible from class loader: " + JLA.getLoaderNameID(ld));
             }
         }
 
diff --git a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
index 39adfb2130a..4720e589efb 100644
--- a/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
+++ b/src/java.base/share/classes/jdk/internal/access/JavaLangAccess.java
@@ -564,4 +564,6 @@ public interface JavaLangAccess {
     StackWalker newStackWalkerInstance(Set<StackWalker.Option> options,
                                        ContinuationScope contScope,
                                        Continuation continuation);
+
+    String getLoaderNameID(ClassLoader loader);
 }

-------------

PR: https://git.openjdk.org/jdk/pull/12641