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

[srikalyan chandrashekar]

On 1/26/14 11:07 AM, Peter Levart wrote:
>
> 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...
Hi Peter, I do not have review rights. So it has to be someone else from 
core-libs-dev.
>
> Regards, Peter

--
Thanks
kalyan

>
>> --
>> 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
>>>>>>>>>
>>>>>>>
>>>>
>>>
>>
>