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

Tue Jan 28 08:44:21 UTC 2014

On 01/28/2014 03:17 AM, David Holmes wrote:
> On 27/01/2014 5: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>
>
> Seems somewhat odd given there is no supertype for Object but it is 
> consistent with the field declaration:
>
> ReferenceQueue<? super T> queue;
>
> The generics here is a little odd as we don't really know the type of 
> T we just play fast-and-loose by declaring:
>
> Reference<Object> r;
>
> Which only works because of erasure. I guess it wouldn't work to try 
> and use a simple wildcard '?' for both 'r' and 'q' as they would be 
> different captures to javac.

Yes, I tried that too and it results in even more unsafe casts.

It's odd yes, since the compile-time error is not present when building 
via OpenJDK build system make files (using "make images" in top 
directory for example) but only if I compile the class from command line 
(using javac directly) or from IDEA. I use JDK 8 ea-b121 in all cases as 
a build JDK. Are there any special options passed to javac for compiling 
those classes in JDK build system that allow such code?

Regards, Peter

>
>> 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...
>
> I can be counted as the Reviewer. Kalyan can be listed as a reviewer.
>
> Thanks Peter.
>
> David
> -----
>
>> 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
>>>>>>>>>>
>>>>>>>>
>>>>>
>>>>
>>>
>>