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

[serguei.spitsyn at oracle.com]

Alan,

This is a webrev that includes the jtreg test:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8156147-Jigsaw-agents.jdk0/


Thanks,
Serguei


On 5/6/16 00:29, serguei.spitsyn at oracle.com wrote:
> 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.spitsyn at 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
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160506/a25cf4e7/attachment.html>