[PATCH] 6822627: NPE at ReferenceTypeImpl.constantPool

[Egor Ushakov]

Yes, all correct, thanks!


On 20.07.2016 12:34, serguei.spitsyn at oracle.com wrote:
> 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
-- 
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/3bbd4e62/attachment-0001.html>