From e5c1052c2070cc3d521abc933b1f5496062d9cad Mon Sep 17 00:00:00 2001 From: mchung Date: Mon, 8 Jul 2013 14:05:59 +0200 Subject: [PATCH] 8014890: (ref) Reference queues may return more entries than expected Summary: When enqueuing references check whether the j.l.r.Reference has already been enqeued or removed in the lock. Do not enqueue them again. This occurs because multiple threads may try to enqueue the same j.l.r.Reference at the same time. Reviewed-by: mchung, dholmes, plevart, shade Contributed-by: thomas.schatzl@oracle.com --- .../classes/java/lang/ref/Reference.java | 6 +- .../classes/java/lang/ref/ReferenceQueue.java | 32 ++++---- test/java/lang/ref/EnqueuePollRace.java | 73 +++++++++++++++++++ 3 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 test/java/lang/ref/EnqueuePollRace.java diff --git a/src/share/classes/java/lang/ref/Reference.java b/src/share/classes/java/lang/ref/Reference.java index bc24e9df0..1cdeefd99 100644 --- a/src/share/classes/java/lang/ref/Reference.java +++ b/src/share/classes/java/lang/ref/Reference.java @@ -89,7 +89,7 @@ public abstract class Reference { private T referent; /* Treated specially by GC */ - ReferenceQueue queue; + volatile ReferenceQueue queue; /* When active: NULL * pending: this @@ -225,9 +225,7 @@ public abstract class Reference { * been enqueued */ public boolean isEnqueued() { - synchronized (this) { - return (this.next != null && this.queue == ReferenceQueue.ENQUEUED); - } + return (this.queue == ReferenceQueue.ENQUEUED); } /** diff --git a/src/share/classes/java/lang/ref/ReferenceQueue.java b/src/share/classes/java/lang/ref/ReferenceQueue.java index 00ca9b3e0..a2fad1cbc 100644 --- a/src/share/classes/java/lang/ref/ReferenceQueue.java +++ b/src/share/classes/java/lang/ref/ReferenceQueue.java @@ -55,25 +55,29 @@ public class ReferenceQueue { private long queueLength = 0; boolean enqueue(Reference r) { /* Called only by Reference class */ - synchronized (r) { - if (r.queue == ENQUEUED) return false; - synchronized (lock) { - r.queue = ENQUEUED; - r.next = (head == null) ? r : head; - head = r; - queueLength++; - if (r instanceof FinalReference) { - sun.misc.VM.addFinalRefCount(1); - } - lock.notifyAll(); - return true; + synchronized (lock) { + // Check that since getting the lock this reference hasn't already been + // enqueued (and even then removed) + ReferenceQueue queue = r.queue; + if ((queue == NULL) || (queue == ENQUEUED)) { + return false; + } + assert queue == this; + r.queue = ENQUEUED; + r.next = (head == null) ? r : head; + head = r; + queueLength++; + if (r instanceof FinalReference) { + sun.misc.VM.addFinalRefCount(1); } + lock.notifyAll(); + return true; } } private Reference reallyPoll() { /* Must hold lock */ - if (head != null) { - Reference r = head; + Reference r = head; + if (r != null) { head = (r.next == r) ? null : r.next; r.queue = NULL; r.next = r; diff --git a/test/java/lang/ref/EnqueuePollRace.java b/test/java/lang/ref/EnqueuePollRace.java new file mode 100644 index 000000000..c19a36671 --- /dev/null +++ b/test/java/lang/ref/EnqueuePollRace.java @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* @test + * @bug 8014890 + * @summary Verify that a race between ReferenceQueue.enqueue() and poll() does not occur. + * @author thomas.schatzl@oracle.com + * @run main/othervm -XX:+UseSerialGC -Xmx10M EnqueuePollRace + */ + +import java.lang.ref.*; + +public class EnqueuePollRace { + + public static void main(String args[]) throws Exception { + new WeakRef().run(); + System.out.println("Test passed."); + } + + static class WeakRef { + ReferenceQueue queue = new ReferenceQueue(); + final int numReferences = 100; + final Reference refs[] = new Reference[numReferences]; + final int iterations = 1000; + + void run() throws InterruptedException { + for (int i = 0; i < iterations; i++) { + queue = new ReferenceQueue(); + + for (int j = 0; j < refs.length; j++) { + refs[j] = new WeakReference(new Object(), queue); + } + + System.gc(); // enqueues references into the list of discovered references + + // now manually enqueue all of them + for (int j = 0; j < refs.length; j++) { + refs[j].enqueue(); + } + // and get them back. There should be exactly numReferences + // entries in the queue now. + int foundReferences = 0; + while (queue.poll() != null) { + foundReferences++; + } + + if (foundReferences != refs.length) { + throw new RuntimeException("Got " + foundReferences + " references in the queue, but expected " + refs.length); + } + } + } + } +} -- GitLab