Hi Stuart,
Thank you for help to publish the fix, my pleasure to contribute to
OpenJDK project.
Thanks,
Eric
On 2012/6/28 12:40, Stuart Marks wrote:
I've posted the webrev here:
http://cr.openjdk.java.net/~smarks/yiming.wang/7123972/webrev.0/
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!
s'marks
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
7123972.zip 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
<http://monaco.us.oracle.com/detail.jsf?cr=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
