Patch review request - Test bug 7123972 test/java/lang/annotation/loaderLeak/ fails intermittently

David Holmes david.holmes at
Wed Jun 27 06:19:52 UTC 2012


Ignore my comment about adding the timeouts. As Alan reminded me if the 
test would hang then jtreg will time it out after two minutes anyway.

So this is good to go as far as I am concerned.


On 27/06/2012 7:51 AM, David Holmes wrote:
> Thanks Eric.
> So to summarize:
> - we create a custom classloader, load a class through it and store a
> reference to that Class in a WeakReference
> - we then drop the reference to the loader
> At this point the only reference to the loader is from the Class loaded,
> which in turn is only weakly reachable.
> I must confess that I'm unclear why this test should be failing in its
> original form. The first gc() should remove the strong reference to the
> loader. That leaves a weak cycle: WeakRef -> Class -> Loader -> Class.
> The second gc() should detect the cycle and clear the WeakRef. I guess
> the problem is that depending on which gc is in use the actual gc()
> calls may not in fact induce every possible GC action.
> The fix seems reasonable in that regard - keep gc'ing and finalizing
> till we see the loader is gone, and so the WeakReference should be
> cleared. The original bug would cause a ref to the Class to remain (from
> the annotation) and hence the WeakRef would not be cleared. However, in
> that case the loader would also be strongly referenced and so never
> become finalizable. So if this test was now run against a JDK with the
> original bug, the test would hang. So I think we need to add a timeout
> in there as well.
> David
> On 25/06/2012 6:06 PM, Eric Wang wrote:
>> On 2012/6/21 20:16, David Holmes wrote:
>>> Hi Eric,
>>> On 21/06/2012 8:57 PM, Eric Wang wrote:
>>>> Hi David,
>>>> Thanks for your review, I have updated the code by following your
>>>> suggestion. please see the attachment.
>>>> I'm not sure whether there's a better way to guarantee object finalized
>>>> by GC definitely within the given time. The proposed fix may work in
>>>> most cases but may still throw InterruptException if execution is
>>>> timeout (2 minutes of JTreg by default).
>>> There is no way to guarantee finalization in a specific timeframe, but
>>> if a simple test hasn't executed finalizers in 2 minutes then that in
>>> itself indicates a problem.
>>> Can you post a full webrev for this patch? I don't like seeing it out
>>> of context and am wondering exactly what we are trying to GC here.
>>> David
>>>> Regards,
>>>> Eric
>>>> On 2012/6/21 14:32, David Holmes wrote:
>>>>> Hi Eric,
>>>>> On 21/06/2012 4:05 PM, Eric Wang wrote:
>>>>>> I come from Java SQE team who are interested in regression test bug
>>>>>> fix.
>>>>>> Here is the first simple fix for bug 7123972
>>>>>> <>, Can you please
>>>>>> help
>>>>>> to review and comment? Attachment is the patch Thanks!
>>>>>> This bug is caused by wrong assumption that the GC is started
>>>>>> immediately to recycle un-referenced objects after System.gc() called
>>>>>> one or two times.
>>>>>> The proposed solution is to make sure the un-referenced object is
>>>>>> recycled by GC before checking if the reference is null.
>>>>> Your patch makes its own assumptions - specifically that finalization
>>>>> must eventually run. At a minimum you should add
>>>>> System.runFinalization() calls after the System.gc() inside the loop.
>>>>> Even that is no guarantee in a general sense, though it should work
>>>>> for hotspot.
>>>>> David
>>>>>> Regards,
>>>>>> Eric
>> Hi Alan & David,
>> Thank you for your comments, sorry for reply this mail late as i was
>> just back from the dragon boat holiday.
>> I have updated the code again based on your suggestions: rename the flag
>> variable, increase the sleep time and remove it from problem list.
>> The attachment is the full webrev for this patch, Can you please review
>> again? Thanks a lot!
>> Regards,
>> Eric

More information about the core-libs-dev mailing list