On 2015-12-04 19:16, Kim Barrett wrote:
On Dec 4, 2015, at 2:16 AM, Per Liden <per.li...@oracle.com> wrote:
On 2015-12-02 19:37, Kim Barrett wrote:
Please review this change to PhantomReference processing, changing the
GC-based notification to automatically clear the referent.
[…]
CR:
https://bugs.openjdk.java.net/browse/JDK-8071507
Webrevs:
http://cr.openjdk.java.net/~kbarrett/8071507/jdk.05/
test/java/lang/ref/PhantomReferentClearing.java:
85 // Delete root -> O1, collect, verify P1 notified, P2 not notified.
86 O1 = null;
87 System.gc();
88 if (Q1.remove(ENQUEUE_TIMEOUT) == null) {
89 throw new RuntimeException("P1 not notified by O1 deletion");
90 } else if (Q2.remove(ENQUEUE_TIMEOUT) != null) {
91 throw new RuntimeException("P2 notified by O1 deletion.");
92 }
93
94 // Delete root -> O2, collect. P2 should be notified.
95 O2 = null;
96 System.gc();
97 if (Q2.remove(ENQUEUE_TIMEOUT) == null) {
98 throw new RuntimeException("P2 not notified by O2 deletion");
99 }
The calls to System.gc() isn't guaranteed to do what the test expects here. As
you know, System.gc(), might not actually do anything, depending on which
collector is used and the current circumstances (GC locker held, a concurrent
GC is already in process, etc). To make the test robust I'd suggest you call
System.gc() and check the queue in a loop, and fail after some reasonable
amount of time/iterations.
I'd rather not clutter and uglify this and other tests to deal with a
problem we don't (so far as I can tell) presently actually have. If a
problem does arise, the form of that problem will help guide what the
solution needs to be. I suspect adding something to WhiteBox would be
the proper approach, so that it can be shared with other tests of this
form.
We do actually have this problem today, i.e. there are cases where our
collectors will not do a "full GC" as a result of a call to System.gc().
One example would be if the GC-locker is active. However, it's seems
very unlikely that this particular test would provoking such a condition.
Our current WhiteBox API for FullGC is prone to the same issue. Maybe it
would be a good idea to strengthen that API and used that in the future
for tests like this.
I understand if you don't want to take on this problem here and now,
given that we have other test which is very similar to this one, and
they seem to have worked so far without issues.
cheers,
/Per
