Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently

[Peter Levart]

On 01/25/2014 05:35 AM, srikalyan chandrashekar wrote:
> Hi Peter, if you are a committer would you like to take this further 
> (OR) perhaps david could sponsor this change.

Hi,

Here's new webrev that takes into account Kaylan's and David's review 
comments:

cr.openjdk.java.net/~plevart/jdk9-dev/OOMEInReferenceHandler/webrev.02/


I changed into using Class.forName() instead of Unsafe for class 
preloading and initialization just to be on the safe side regarding 
unwanted premature initialization of Unsafe class. I also took the 
liberty of removing an unneeded semicolon (line 114) and fixing a JDK 8 
compile time error in generics (line 189):

     incompatible types: java.lang.ref.ReferenceQueue<capture#1 of ? 
super java.lang.Object> cannot be converted to 
java.lang.ref.ReferenceQueue<java.lang.Object>

I re-ran the java/lang/ref tests and they pass.

Can I count you as a reviewer, Kalyan? If I get a "go" also from David, 
I'll commit this to jdk9/dev...

Regards, Peter

> --
> Thanks
> kalyan
> On 1/24/14 4:05 PM, Peter Levart wrote:
>>
>> On 01/24/2014 02:53 AM, srikalyan chandrashekar wrote:
>>> Hi David, yes thats right, only benefit i see is we can avoid 
>>> assignment to 'r' if pending is null.
>>
>> Hi Kalyan,
>>
>> Good to hear that test runs without failures so far.
>>
>> Regarding assignment of 'r'. What I tried to accomplish with the 
>> change was eliminate double reading of 'pending' field. I have a 
>> mental model of local variable being a register and field being a 
>> memory location. This may be important if the field is volatile, but 
>> for normal fields, I guess the optimizer knows how to compile such 
>> code most optimally in either case. The old (your) version is better 
>> from logical perspective, since it guarantees that dereferencing the 
>> 'r', wherever it is possible, will never throw NPE (dereferencing 
>> where 'r' is not assigned is not possible because of definitive 
>> assignment rules). So I support going back to your version...
>>
>> Regards, Peter
>>
>>>
>>> -- 
>>> Thanks
>>> kalyan
>>>
>>> On 1/23/14 4:33 PM, David Holmes wrote:
>>>> On 24/01/2014 6:10 AM, srikalyan wrote:
>>>>> Hi Peter, i have modified your code from
>>>>>
>>>>> r = pending;
>>>>> if (r != null) {
>>>>>       ......
>>>>>        TO
>>>>> if (pending != null) {
>>>>>       r = pending;
>>>>>
>>>>> This is because the r is used later in the code and must not be 
>>>>> assigned
>>>>> pending unless it is not null(this was as is earlier).
>>>>
>>>> If r is null, because pending is null then you perform the wait() 
>>>> and then continue - back to the top of the loop. There is no bug in 
>>>> Peter's code.
>>>>
>>>> The new webrev is
>>>>> posted here
>>>>> http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev-V2/ 
>>>>>
>>>>> . I ran a 1000 run and no failures so far, however i would like to 
>>>>> run a
>>>>> couple more 1000 runs to assert the fix.
>>>>>
>>>>> PS: The description section of JEP-122
>>>>> (http://openjdk.java.net/jeps/122) says meta-data would be in native
>>>>> memory(not heap).
>>>>
>>>> The class_mirror is a Java object not meta-data.
>>>>
>>>> David
>>>>
>>>>> -- 
>>>>> Thanks
>>>>> kalyan
>>>>> Ph: (408)-585-8040
>>>>>
>>>>>
>>>>> On 1/21/14, 2:31 PM, Peter Levart wrote:
>>>>>>
>>>>>> On 01/21/2014 07:17 PM, srikalyan wrote:
>>>>>>> Hi Peter/David, catching up after long weekend. Why would there 
>>>>>>> be an
>>>>>>> OOME in object heap due to class loading in perm gen space ?
>>>>>>
>>>>>> The perm gen is not a problem her (JDK 8 does not have it and we see
>>>>>> OOME on JDK8 too). Each time a class is loaded, new java.lang.Class
>>>>>> object is allocated on heap.
>>>>>>
>>>>>> Regards, Peter
>>>>>>
>>>>>>> Please correct if i am missing something here. Meanwhile i will 
>>>>>>> give
>>>>>>> the version of Reference Handler you both agreed on a try.
>>>>>>> -- 
>>>>>>> Thanks
>>>>>>> kalyan
>>>>>>> Ph: (408)-585-8040
>>>>>>>
>>>>>>> On 1/21/14, 7:24 AM, Peter Levart wrote:
>>>>>>>> On 01/21/2014 07:54 AM, Peter Levart wrote:
>>>>>>>>> *[Loaded sun.misc.Cleaner from
>>>>>>>>> /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]*
>>>>>>>>> [Loaded java.io.ByteArrayInputStream from
>>>>>>>>> /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
>>>>>>>>> [Loaded sun.util.calendar.ZoneInfoFile$ZoneOffsetTransitionRule
>>>>>>>>> from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'm on linux, 64bit and using official EA build 121 of JDK 8...
>>>>>>>>>
>>>>>>>>> But if I try with JDK 7u45, I don't see it.
>>>>>>>>
>>>>>>>> So what changed between JDK 7 and JDK 8?
>>>>>>>>
>>>>>>>> I suspect the following: 8007572: Replace existing jdk timezone 
>>>>>>>> data
>>>>>>>> at <java.home>/lib/zi with JSR310's tzdb
>>>>>>>>
>>>>>>>>
>>>>>>>> Regards, Peter
>>>>>>>>
>>>>>>
>>>
>>
>