[PATCH] 6822627: NPE at ReferenceTypeImpl.constantPool

[serguei.spitsyn at oracle.com]

Egor,

If I understand correctly, you do not have an openJdk user ID and an 
author status.
Please, see the link:
   http://openjdk.java.net/census

So that, I'll commit your fix with the comment:
   Contributed-by: egor.ushakov at jetbrains.com

and push it to the jdk9/hs.

I hope, somebody will correct me if it is not right.
Please, let me know if it works for you.

Thanks,
Serguei


On 7/20/16 02:10, Egor Ushakov wrote:
>
> Serguei, thanks for the review!
>
> Please sponsor the fix, I do not know how to proceed.
>
> Thanks!
>
> Egor
>
>
> On 20.07.2016 10:36, serguei.spitsyn at oracle.com wrote:
>> Hi Egor,
>>
>> Thank you for providing the test!
>>
>> A couple of nits to the test:
>>
>>    56             if (!Arrays.equals(cpbytes, cpbytes2)) {
>>    57                 failure("Consequent constantPool results vary, first was : " + cpbytes + ", now: " + cpbytes2);
>>    58             };
>>    Last semicolon is not need.
>>    74         if (!testFailed) {
>>    75             println("ConstantPoolInfoGC: passed");
>>    76         } else {
>>    77             throw new Exception("ConstantPoolInfoGC: failed");
>>    78         }
>>   I'd suggest to rearrange the statement above to something like this:
>>    74         if (testFailed) {
>>    75             throw new Exception("ConstantPoolInfoGC: failed");
>>    76         }
>>    77         println("ConstantPoolInfoGC: passed");
>>
>>    But I leave it up to you.    No need to see another webrev.    I 
>> can sponsor your fix, if needed. Thanks, Serguei On 7/19/16 00:07, 
>> Egor Ushakov wrote:
>>>
>>> Hi Serguei,
>>>
>>> here's a new webrev with a test included: 
>>> http://cr.openjdk.java.net/~avu/JDK-6822627/webrev.01/
>>>
>>> Egor
>>>
>>> On 15.07.2016 12:22, serguei.spitsyn at oracle.com wrote:
>>>> Hi Egor, The fix looks good modulo the synchronization issue. Do 
>>>> you have a jtreg unit test reproducing this failure mode? We have a 
>>>> rule to provide a test coverage for the fixes. Thanks, Serguei On 
>>>> 7/15/16 00:03, Egor Ushakov wrote:
>>>>>
>>>>> Thanks for your comments!
>>>>>
>>>>> I totally agree that the code there requires major refactoring. In 
>>>>> the fix I tried not to make it worse and fix the NPE.
>>>>>
>>>>> On 14.07.2016 20:06, Martin Buchholz wrote:
>>>>>> The lack of volatile or synchronization, plus the use of multiple 
>>>>>> mutable fields, suggests that a deeper fix is necessary to be 
>>>>>> truly correct.  For comparison, much of the similar code 
>>>>>> implementing reflection has been changed to use volatile.  But 
>>>>>> that's a big job, for the serviceability team.
>>>>>> On Thu, Jul 14, 2016 at 2:33 AM, Egor Ushakov 
>>>>>> <egor.ushakov at jetbrains.com <mailto:egor.ushakov at jetbrains.com>> 
>>>>>> wrote:
>>>>>>
>>>>>>     Hi, I'm a developer from IDEA debugger (we have company OCA).
>>>>>>     We've increased usage of ReferenceTypeImpl.constantPool and
>>>>>>     now facing JDK-6822627 more often. Please review and sponsor
>>>>>>     the fix. The problem was that constantPoolBytesRef
>>>>>>     SoftReference value coud be GCed and there was no check for
>>>>>>     that. I've added it to the check inside getConstantPoolInfo
>>>>>>     to request the bytes again in this case. BUGURL:
>>>>>>     https://bugs.openjdk.java.net/browse/JDK-6822627 WEBREV:
>>>>>>     http://cr.openjdk.java.net/~avu/JDK-6822627/webrev.00/
>>>>>>     <http://cr.openjdk.java.net/%7Eavu/JDK-6822627/webrev.00/>
>>>>>>     Thanks!-- Egor Ushakov Software Developer JetBrains
>>>>>>     http://www.jetbrains.com The Drive to Develop 
>>>>>>
>>>>> -- 
>>>>> Egor Ushakov
>>>>> Software Developer
>>>>> JetBrains
>>>>> http://www.jetbrains.com
>>>>> The Drive to Develop
>>> -- 
>>> Egor Ushakov
>>> Software Developer
>>> JetBrains
>>> http://www.jetbrains.com
>>> The Drive to Develop
> -- 
> Egor Ushakov
> Software Developer
> JetBrains
> http://www.jetbrains.com
> The Drive to Develop
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20160720/9458b7d3/attachment.html>