RFR (XS): 8014890 : Reference queues may return more entries than expected (original) (raw)

David Holmes david.holmes at oracle.com
Wed Jun 19 02:47:19 UTC 2013

Hi Thomas,

On 19/06/2013 7:08 AM, Thomas Schatzl wrote:

Hi all,

can I have reviews for the following change? It happens if multiple threads are enqueuing and dequeuing reference objects into a reference queue, that Reference objects may be enqueued at multiple times. This is because when java.lang.ref.ReferenceQueue.poll() returns and inactivates a Reference object, another thread may just be during enqueuing it again.

Are we talking about different queues here? Otherwise both poll() and enqueue() are using synchronized(lock). I also note that enqueue synchronizes on the Reference itself, which suggests that poll (actually reallyPoll) should also be synchronizing on the reference (though we have a nested lock inversion problem that would need to be solved).

David

In the test case provided, the two threads that conflict are the reference handler thread and the program (main) thread.

Relevant code: ReferenceHandlerThread.run(): 0: [...] 1: ReferenceQueue q = r.queue; // r is the reference 2: if (r != ReferenceQueue.NULL) 3: q.enqueue(). ReferenceQueue::poll()(reallyPoll()) code (I removed lots of code here) 1: synchronized(lock) { 2: [...] 3: r.queue = ReferenceQueue.NULL; 3:} I.e. while ReferenceQueue.poll() sets the Reference's queue to the NULL queue so that that reference will not be enqueued again (or at most into the NULL queue which is a nop), it happens that if the task switch occurs between lines 2 and 3 of the reference handler thread, q still contains the old queue reference, and the reference handler thread will enqueue the Reference into the original queue again. You can achieve the same effect by simply calling ReferenceQueue.enqueue() (i.e. without the reference handler thread, or within the reference handler thread doing the != NULL check), it's just that in such a case the "old" ReferenceQueue is stored in some register. The guard for ReferenceQueue.NULL does not have any effect except for possibly saving the virtual call. Simply calling r.enqueue() exhibits the same problem. The proposed solution is to filter out References within ReferenceQueue.enqueue() again. At that point we can check whether the given Reference is actually meant for this queue or not. Already removed References are known to be "inactive" (as enqueue and poll are mutually exclusive using a lock), in particular the References' queue is different (actually the NULL queue) to the queue it is being enqueued. This change should pose no change in semantics, as the ReferenceQueue of the Reference can only be set in its constructor, and as soon as the Reference is removed from a ReferenceQueue, its ReferenceQueue will be the NULL queue. (I.e. before this change you could not enqueue such an "inactive" Reference multiple times anyway) (too many References and queues here :) Webrev with test case http://cr.openjdk.java.net/~tschatzl/8014890/webrev/ JIRA: https://jbs.oracle.com/bugs/browse/JDK-8014890 bugs.sun.com http://bugs.sun.com/viewbug.do?bugid=8014890 Testing: jck, jprt, manual testing Note that I also need a sponsor to push in case this change is approved. Thanks, Thomas

More information about the core-libs-dev mailing list