RFR: 8072777: java/lang/ref/ReferenceEnqueuePending.java: some references aren't queued
peter.levart at gmail.com
Sun Jan 31 09:19:18 UTC 2016
This change will make it practically impossible to miss the enqueued
references. But this could be an opportunity to also clean-up the code a
bit. The checkResult method has an 'obj' parameter whose purpose is not
immediately obvious. It's only used in the following line:
136 // Use the last Reference object of those created above,
so as to keep it "alive".
137 System.out.println("I must write " + obj + " to prevent
...the comment is misleading. If we look at how checkResult is called,
we can see that it is not the last Reference object that is passed to
this parameter, but its referent (an Integer object). This is the
referent of the Reference that is not supposed to be enqueued as
checkResult is called with parameters (refQueue, obj, iterations-1),
which means that it is expected to retrieve one less Reference object
than the number of Reference objects created.
I don't know why such complications are necessary. Is the test supposed
to also verify the requirement that a Reference whose referent is kept
strongly reachable will not be enqueued? If that is true, then there are
two things that should be fixed to ensure this requirement is actually
- the last Reference object that contains a referent that is kept
strongly reachable should also be kept strongly reachable while the
checkResult method is dequeuing References or else it can be GCed before
actually being discovered.
- the checkReference method should also wait for some time for the last
Reference to be enqueued. It is not necessary to wait 30 seconds. 2
seconds should be enough to not miss this last Reference if it was
enqueued as a result of some bug.
As to the implementation, it is not necessary to pass the last Reference
or it's referent to the checkReference method to keep them strongly
reachable. checkReference method does not have to deal with that. The
last Reference and it's referent could be kept strongly reachable in the
102 // Verify that all WeakReference objects (but last one)
ended up queued.
103 checkResult(refQueue, iterations-1);
// Keep the last Reference object and its referent
alive until after the ckeck
104 System.out.println("Test passed.");
On 01/31/2016 05:37 AM, Kim Barrett wrote:
> Please review this fix of a bug in the ReferenceEnqueuePending test.
> The original code assumes the pending list processor will stay ahead
> of the test thread, so that the test's polling of the reference queue
> won't find it empty while there are still references on the internal
> pending list. That might not always happen though, possibly in part
> because the test raises the priority of its own thread to better
> compete with the pending list processor.
> The fix is to use queue.remove with a large timeout, rather than
> queue.poll, to pull references from the queue until we've obtained all
> we expect to get. Then switch to polling to verify nothing unexpected
> in the queue without making the test delay for an expected timeout. If
> all of the references are (eventually) queued as expected then the
> timeout will never come close to expiring.
> I also looked at uses of poll() by other tests in the same directory,
> and didn't find any that looked like they might have a similar problem
> of failing because the pending list processor had not yet gotten
> around to dealing with a reference.
> Local testing of before and after test code, with an occasional sleep
> inserted into the pending list processor's loop to simulate a
> scheduling delay. With that delay, the original test frequently fails,
> while the modified test consistently passes.
More information about the core-libs-dev