On 7/20/16 02:53, Egor Ushakov wrote:

Yes, all correct, thanks!

Ok, thanks.
Serguei



On 20.07.2016 12:34, serguei.spit...@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.usha...@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.spit...@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.spit...@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.usha...@jetbrains.com <mailto:egor.usha...@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

Reply via email to