Re: RFR (XS): 8156147 NPE in InstrumentationImpl.transform when loading classes from -Xbootclasspath/a

[serguei.spit...@oracle.com]

On 5/6/16 00:23, Alan Bateman wrote:

This looks okay to me except I don't see a test. I think we will need 
test coverage for instrumenting classes in the unnamed module of the 
boot loader.

Ok, I'll check if some of the jdk_instrument tests can be modified to 
add this coverage.



Also can you push this to jdk9/dev? We have a number of people on 
jigsaw-dev reporting this issue and would be good to get it fixed in 
jdk-9+118.

Sure, I'll rebase the fix to the jdk9/dev.


Thanks,
Serguei



-Alan



On 06/05/2016 08:18, serguei.spit...@oracle.com wrote:
Please, review a simple fix in the 
java.lang.instrument.InstrumentationImpl transform() method.

This is the patch:

diff -r d2f46fdfc3ca src/java.base/share/classes/module-info.java
--- a/src/java.base/share/classes/module-info.java    Thu May 05 
11:44:01 2016 -0700
+++ b/src/java.base/share/classes/module-info.java    Fri May 06 
00:11:23 2016 -0700
@@ -145,6 +145,8 @@
         jdk.scripting.nashorn;
     exports jdk.internal.org.objectweb.asm.signature to
         jdk.scripting.nashorn;
+    exports jdk.internal.loader to
+        java.instrument;
     exports jdk.internal.math to
         java.desktop;
     exports jdk.internal.module to


diff -r d2f46fdfc3ca 
src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java
--- 
a/src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java 
Thu May 05 11:44:01 2016 -0700
+++ 
b/src/java.instrument/share/classes/sun/instrument/InstrumentationImpl.java 
Fri May 06 00:11:23 2016 -0700
@@ -41,6 +41,8 @@
 import java.util.Objects;
 import java.util.jar.JarFile;

+import jdk.internal.loader.BootLoader;
+
 /*
  * Copyright 2003 Wily Technology, Inc.
  */
@@ -436,7 +438,8 @@
             if (classBeingRedefined != null) {
                 module = classBeingRedefined.getModule();
             } else {
-                module = loader.getUnnamedModule();
+                module = (loader == null) ? 
jdk.internal.loader.BootLoader.getUnnamedModule()
+                                          : loader.getUnnamedModule();
             }
         }
         if (mgr == null) {


Summary:

   InstrumentationImpl.transform has this:
        if (module == null) {
            if (classBeingRedefined != null) {
                module = classBeingRedefined.getModule();
            } else {
                module = loader.getUnnamedModule();
            }
        }

   but if loader is null (-Xbootclasspath/a case) then this throws 
NullPointerException.
   If loader is null then we need to use 
jdk.internal.loader.BootLoader.getUnnamedModule().


Testing:
   In progress: run the JTreg :jdk_instrument tests.


Thanks,
Serguei