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

Stuart Marks stuart.marks at
Thu Jun 28 04:40:13 UTC 2012

I've posted the webrev here:

I've looked at the changes and they seem fine.

It's interesting that the run times take 30-60 sec in your experience. I've run 
them on my system (Linux in a virtual machine) and they usually run in 10-20 
sec. In any case, as you say, if the test times out it indicates there really 
was a failure.

I'll give people a chance to look at the webrev and if there aren't any more 
comments in another day or so I'll push in the changeset.

Thanks for developing this!


On 6/26/12 11:50 PM, Eric Wang wrote:
> Hi David,
> Thank you! I run the test several times on 3 platforms (windows, solaris and
> linux), the average execution time is 30secs to 60secs. if the test hang 2
> minutes, there should be something wrong.
> Hi Marks,
> I don't have the author role, Can you please help to review and post the webrev
> in attachment if it is OK, Thanks a lot!
> Regards,
> Eric
> On 2012/6/27 14:19, David Holmes wrote:
>> Eric,
>> 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.
>> Thanks,
>> David
>> 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