8166289: RuntimeException: canRead() reports false for reading from the same module: expected true, was false

A fix in the JDWP test along with some extra logging added

Reviewed-by: sspitsyn

hotspot/test/serviceability/jdwp/AllModulesCommandTest.java

@@ -30,8 +30,9 @@ import static jdk.test.lib.Asserts.assertTrue;
 
 /**
  * @test
- * @summary Tests AllModules JDWP command
+ * @summary Tests the modules-related JDWP commands
  * @library /test/lib
+ * @modules jdk.jdwp.agent
  * @modules java.base/jdk.internal.misc
  * @compile AllModulesCommandTestDebuggee.java
  * @run main/othervm AllModulesCommandTest
@@ -87,11 +88,16 @@ public class AllModulesCommandTest implements DebuggeeLauncher.Listener {
             assertReply(reply);
             for (int i = 0; i < reply.getModulesCount(); ++i) {
                 long modId = reply.getModuleId(i);
-                // For each module reported by JDWP get its name using the JDWP  NAME command
+                // For each module reported by JDWP get its name using the JDWP NAME command
-                getModuleName(modId);
+                // and store the reply
+                String modName = getModuleName(modId);
+                System.out.println("i=" + i + ", modId=" + modId + ", modName=" + modName);
+                if (modName != null) { // JDWP reports unnamed modules, ignore them
+                    jdwpModuleNames.add(modName);
+                }
                 // Assert the JDWP CANREAD and CLASSLOADER commands
-                assertCanRead(modId);
+                assertCanRead(modId, modName);
-                assertClassLoader(modId);
+                assertClassLoader(modId, modName);
             }
 
             System.out.println("Module names reported by JDWP: " + Arrays.toString(jdwpModuleNames.toArray()));
@@ -114,14 +120,10 @@ public class AllModulesCommandTest implements DebuggeeLauncher.Listener {
         }
     }
 
-    private void getModuleName(long modId) throws IOException {
+    private String getModuleName(long modId) throws IOException {
-        // Send out the JDWP NAME command and store the reply
         JdwpModNameReply reply = new JdwpModNameCmd(modId).send(channel);
         assertReply(reply);
-        String modName = reply.getModuleName();
+        return reply.getModuleName();
-        if (modName != null) { // JDWP reports unnamed modules, ignore them
-            jdwpModuleNames.add(modName);
-        }
     }
 
     private void assertReply(JdwpReply reply) {
@@ -131,19 +133,47 @@ public class AllModulesCommandTest implements DebuggeeLauncher.Listener {
         }
     }
 
-    private void assertCanRead(long modId) throws IOException {
+    private void assertCanRead(long modId, String modName) throws IOException {
         // Simple assert for the CANREAD command
         JdwpCanReadReply reply = new JdwpCanReadCmd(modId, modId).send(channel);
         assertReply(reply);
-        assertTrue(reply.canRead(), "canRead() reports false for reading from the same module");
+        assertTrue(reply.canRead(), "canRead() reports false for reading from the same module '" + modName + "', moduleId=" + modId);
     }
 
-    private void assertClassLoader(long modId) throws IOException {
+    private void assertClassLoader(long modId, String modName) throws IOException {
-        // Simple assert for the CLASSLOADER command
+        // Verify that the module classloader id is valid
         JdwpClassLoaderReply reply = new JdwpClassLoaderCmd(modId).send(channel);
         assertReply(reply);
-        long clId = reply.getClassLoaderId();
+        long moduleClassLoader = reply.getClassLoaderId();
-        assertTrue(clId >= 0, "bad classloader refId " + clId + " for module id " + modId);
+        assertTrue(moduleClassLoader >= 0, "bad classloader refId " + moduleClassLoader + " for module '" + modName + "', moduleId=" + modId);
+
+        String clsModName = getModuleName(modId);
+        if ("java.base".equals(clsModName)) {
+            // For the java.base module, because there will be some loaded classes, we can verify
+            // that some of the loaded classes do report the java.base module as the module they belong to
+            assertGetModule(moduleClassLoader, modId);
+        }
+    }
+
+    private void assertGetModule(long moduleClassLoader, long modId) throws IOException {
+        // Get all the visible classes for the module classloader
+        JdwpVisibleClassesReply visibleClasses = new JdwpVisibleClassesCmd(moduleClassLoader).send(channel);
+        assertReply(visibleClasses);
+
+        boolean moduleFound = false;
+        for (long clsId : visibleClasses.getVisibleClasses()) {
+            // For each visible class get the module the class belongs to
+            JdwpModuleReply modReply = new JdwpModuleCmd(clsId).send(channel);
+            assertReply(modReply);
+            long clsModId = modReply.getModuleId();
+
+            // At least one of the visible classes should belong to our module
+            if (modId == clsModId) {
+                moduleFound = true;
+                break;
+            }
+        }
+        assertTrue(moduleFound, "None of the visible classes for the classloader of the module " + getModuleName(modId) + " reports the module as its own");
     }
 
 }

hotspot/test/serviceability/jdwp/JdwpCanReadReply.java

@@ -31,7 +31,7 @@ public class JdwpCanReadReply extends JdwpReply {
     private boolean canRead;
 
     protected void parseData(DataInputStream ds) throws IOException {
-        canRead = ds.read() == 1;
+        canRead = (ds.read() != 0);
     }
 
     public boolean canRead() {

hotspot/test/serviceability/jdwp/JdwpCmd.java

@@ -70,7 +70,6 @@ public abstract class JdwpCmd<T extends JdwpReply> {
     }
 
     public final T send(JdwpChannel channel) throws IOException {
-        System.err.println("Sending command: " + this);
         channel.write(data.array(), HEADER_LEN + getDataLength());
         if (reply != null) {
             reply.initFromStream(channel.getInputStream());

hotspot/test/serviceability/jdwp/JdwpModuleCmd.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+/**
+ * The JDWP MODULE command
+ */
+public class JdwpModuleCmd extends JdwpCmd<JdwpModuleReply> {
+
+    public JdwpModuleCmd(long refId) {
+        super(19, 2, JdwpModuleReply.class, refLen());
+        putRefId(refId);
+    }
+
+}

hotspot/test/serviceability/jdwp/JdwpModuleReply.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+import java.io.DataInputStream;
+import java.io.IOException;
+
+/**
+ * The reply to the JDWP MODULE command
+ */
+public class JdwpModuleReply extends JdwpReply {
+
+    private long moduleId;
+
+    protected void parseData(DataInputStream ds) throws IOException {
+        moduleId = readRefId(ds);
+    }
+
+    public long getModuleId() {
+        return moduleId;
+    }
+
+}

hotspot/test/serviceability/jdwp/JdwpVisibleClassesCmd.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+/**
+ * The JDWP VISIBLE CLASSES command
+ */
+public class JdwpVisibleClassesCmd extends JdwpCmd<JdwpVisibleClassesReply> {
+
+    public JdwpVisibleClassesCmd(long classLoaderId) {
+        super(1, 14, JdwpVisibleClassesReply.class, refLen());
+        putRefId(classLoaderId);
+    }
+
+}

hotspot/test/serviceability/jdwp/JdwpVisibleClassesReply.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2016, 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.
+ */
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.util.Arrays;
+
+/**
+ * The reply to the JDWP VISIBLE CLASSES command
+ */
+public class JdwpVisibleClassesReply extends JdwpReply {
+
+    private long[] visibleClasses;
+
+    protected void parseData(DataInputStream ds) throws IOException {
+        int numOfClasses = ds.readInt();
+        visibleClasses = new long[numOfClasses];
+        for (int i = 0; i < numOfClasses; ++i) {
+            byte type = ds.readByte();
+            long refId = readRefId(ds);
+            visibleClasses[i] = refId;
+        }
+    }
+
+    public long[] getVisibleClasses() {
+        return Arrays.copyOf(visibleClasses, visibleClasses.length);
+    }
+
+}