Hi Stuart,
I don't think finalization or reference queues are needed in this case.
We could simply do:
while(true) {
System.gc();
if (weakRef.get() == null)
break;
}
We have to remember that this is not testing the correct operation of
weak references, nor of finalization, but simply that no unintended
strong references remain to the object referenced via the WeakReference.
So if a strong ref remains, or somehow weakrefs are broken, then the
loop will not terminate on its own and we rely on the test harness
timing it out.
David
-----
On 29/06/2012 7:20 AM, Stuart Marks wrote:
And here's the webrev for this one:
http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.0/
Also looks good to me. It's pretty similar to the other fix (7123972).
If there are no further comments I'll push this in the next day or so.
* * *
Some further comments, mainly aimed at the likes of David Holmes. I'm
mostly "thinking out loud" at the moment.
While I'm glad to see the reliability of these tests improved and to see
them come off the problem list, it would be nice to see something more
general that could be shared among other tests, and possibly more
abstract so that things can be tuned to different VM characteristics if
necessary.
Basically what these tests want to do is to make sure that a certain
object gets garbage collected. I think this is pretty common. There are
several dozen tests in the jdk repo that have the word "leak" in them. I
bet they all use different techniques to detect leaks. :-(
The technique used here is to get a weak reference to the object, and
then use the object's finalizer to set a bit telling us when
finalization has occurred. At this point we can assert that the weak ref
has been cleared. In addition to being somewhat roundabout, this also
seems like we're merely cross-checking the garbage collector. We could
easily poll for the weak ref being cleared, and then assert that the bit
set by finalization has indeed been set.
It seems to me we could avoid finalization entirely and just use weak
refs and reference queues. Would it have the same effect and semantics
as the current test if we were to do the following?
- with a target object, make a weak reference registered with a ref queue
- clear all references to the target object
- request gc
- request finalization (is this necessary?)
- wait or poll on the ref queue
If we get our weak ref from the queue, success. If we time out, fail.
Note that there is a timeout mechanism ReferenceQueue.remove(timeout),
so we could set some timeout without waiting the full jtreg timeout if
we don't want to. Or we could call System.gc() in a loop using remove()
with a short timeout (instead of sleep), similar to the current test,
although it's not clear to me this is necessary.
s'marks
On 6/27/12 1:20 AM, Eric Wang wrote:
Hi David,
Thank you for review!
Hi Stuart,
Can you please help to review and post the webrev, Thanks a lot!
Eric
On 2012/6/27 15:32, David Holmes wrote:
On 27/06/2012 4:57 PM, Eric Wang wrote:
Hi David & Stuart,
Thank you for the help! please review the in webrev 6948101.zip in
attachment.
Thanks Eric. That fix also seems fine to me.
David
Regards,
Eric
On 2012/6/27 9:14, Stuart Marks wrote:
Hi Eric,
Alan Bateman asked me to help you out with this since he's going to be
unavailable for a couple weeks.
I didn't see you on the OpenJDK census [1] so you might not have an
Author role and thus the ability to post webrevs. If indeed you don't,
I can post a webrev for you. I can also post a webrev for your other
review (7123972) if it's still necessary.
Finally, I can push the fixes for you when the reviews are complete.
s'marks
[1] http://openjdk.java.net/census
On 6/26/12 2:56 PM, David Holmes wrote:
Hi Eric,
On 26/06/2012 7:26 PM, Eric Wang wrote:
Please help to review the fix attached for test bug 6948101
<http://monaco.us.oracle.com/detail.jsf?cr=6948101> which is same
root
cause as bug 7123972
<http://monaco.us.oracle.com/detail.jsf?cr=7123972>.
The test makes wrong assumption that GC is started immediately to
recycle unused objects after System.gc() called.
The proposed fix is to make sure objects have been recycled by GC
before
checking if the weak reference is null.
Again I really need to see a webrev to see the fix in context. Do you
have
Author role and an OpenJDK user name so you can post webrevs on
cr.openjdk.java.net?
I suspect this may have the same issues as the other fix and require
a timeout
for when the original problem is still present.
Thanks,
David
Regards,
Eric
