4243978: (ref) Race condition in Reference.enqueue()
4268317: (ref) Reference.isEnqueued() can return true when instance not enqueued
Summary: The reference handler now declares, and assumes, that the discovered field, rather than the next field, is (to be) used to link the entries in the pending list, thus allowing a reference object to be safely enqueued even while it is in the pending state. Also added slightly modified regression tests from the two bug reports.
Reviewed-by: mchung, alanb, jcoomes
--- a/jdk/src/share/classes/java/lang/ref/Reference.java Mon Oct 31 16:23:43 2011 -0700
+++ b/jdk/src/share/classes/java/lang/ref/Reference.java Mon Oct 31 17:38:15 2011 -0700
@@ -27,7 +27,6 @@
import sun.misc.Cleaner;
-
/**
* Abstract base class for reference objects. This class defines the
* operations common to all reference objects. Because reference objects are
@@ -69,7 +68,7 @@
* null.
*
* Pending: queue = ReferenceQueue with which instance is registered;
- * next = Following instance in queue, or this if at end of list.
+ * next = this
*
* Enqueued: queue = ReferenceQueue.ENQUEUED; next = Following instance
* in queue, or this if at end of list.
@@ -81,17 +80,28 @@
* the next field is null then the instance is active; if it is non-null,
* then the collector should treat the instance normally.
*
- * To ensure that concurrent collector can discover active Reference
+ * To ensure that a concurrent collector can discover active Reference
* objects without interfering with application threads that may apply
* the enqueue() method to those objects, collectors should link
- * discovered objects through the discovered field.
+ * discovered objects through the discovered field. The discovered
+ * field is also used for linking Reference objects in the pending list.
*/
private T referent; /* Treated specially by GC */
ReferenceQueue<? super T> queue;
+ /* When active: NULL
+ * pending: this
+ * Enqueued: next reference in queue (or this if last)
+ * Inactive: this
+ */
Reference next;
+
+ /* When active: next element in a discovered reference list maintained by GC (or this if last)
+ * pending: next element in the pending list (or null if last)
+ * otherwise: NULL
+ */
transient private Reference<T> discovered; /* used by VM */
@@ -106,7 +116,8 @@
/* List of References waiting to be enqueued. The collector adds
* References to this list, while the Reference-handler thread removes
- * them. This list is protected by the above lock object.
+ * them. This list is protected by the above lock object. The
+ * list uses the discovered field to link its elements.
*/
private static Reference pending = null;
@@ -120,14 +131,12 @@
public void run() {
for (;;) {
-
Reference r;
synchronized (lock) {
if (pending != null) {
r = pending;
- Reference rn = r.next;
- pending = (rn == r) ? null : rn;
- r.next = r;
+ pending = r.discovered;
+ r.discovered = null;
} else {
try {
lock.wait();
@@ -201,10 +210,8 @@
* been enqueued
*/
public boolean isEnqueued() {
- /* In terms of the internal states, this predicate actually tests
- whether the instance is either Pending or Enqueued */
synchronized (this) {
- return (this.queue != ReferenceQueue.NULL) && (this.next != null);
+ return (this.next != null && this.queue == ReferenceQueue.ENQUEUED);
}
}
--- a/jdk/src/share/javavm/export/jvm.h Mon Oct 31 16:23:43 2011 -0700
+++ b/jdk/src/share/javavm/export/jvm.h Mon Oct 31 17:38:15 2011 -0700
@@ -1424,7 +1424,8 @@
*/
unsigned int thread_park_blocker : 1;
unsigned int post_vm_init_hook_enabled : 1;
- unsigned int : 30;
+ unsigned int pending_list_uses_discovered_field : 1;
+ unsigned int : 29;
unsigned int : 32;
unsigned int : 32;
} jdk_version_info;
--- a/jdk/src/share/native/common/jdk_util.c Mon Oct 31 16:23:43 2011 -0700
+++ b/jdk/src/share/native/common/jdk_util.c Mon Oct 31 17:38:15 2011 -0700
@@ -101,5 +101,5 @@
// Advertise presence of sun.misc.PostVMInitHook:
// future optimization: detect if this is enabled.
info->post_vm_init_hook_enabled = 1;
-
+ info->pending_list_uses_discovered_field = 1;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/ref/ReferenceEnqueue.java Mon Oct 31 17:38:15 2011 -0700
@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2011, 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 4268317
+ * @summary Test if Reference.enqueue() works properly with GC
+ */
+
+import java.lang.ref.*;
+
+public class ReferenceEnqueue {
+
+ public static void main(String args[]) throws Exception {
+ for (int i=0; i < 5; i++)
+ new WeakRef().run();
+ System.out.println("Test passed.");
+ }
+
+ static class WeakRef {
+ final ReferenceQueue<Object> queue = new ReferenceQueue<Object>();
+ final Reference<Object> ref;
+ final int iterations = 1000;
+
+ WeakRef() {
+ this.ref = new WeakReference<Object>(new Object(), queue);
+ }
+
+ void run() throws InterruptedException {
+ System.gc();
+ for (int i = 0; i < iterations; i++) {
+ System.gc();
+ if (ref.isEnqueued()) {
+ break;
+ }
+
+ Thread.sleep(100);
+ }
+
+ if (ref.isEnqueued() == false) {
+ // GC have not enqueued refWeak for the timeout period
+ System.out.println("Reference not enqueued yet");
+ return;
+ }
+
+ if (ref.enqueue() == true) {
+ // enqueue() should return false since
+ // ref is already enqueued by the GC
+ throw new RuntimeException("Error: enqueue() returned true;"
+ + " expected false");
+ }
+
+ if (queue.poll() == null) {
+ // poll() should return ref enqueued by the GC
+ throw new RuntimeException("Error: poll() returned null;"
+ + " expected ref object");
+ }
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/lang/ref/ReferenceEnqueuePending.java Mon Oct 31 17:38:15 2011 -0700
@@ -0,0 +1,201 @@
+/* Copyright (c) 2011, 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 4243978
+ * @summary Test if Reference.enqueue() works properly with pending references
+ */
+import java.lang.ref.*;
+
+public class ReferenceEnqueuePending {
+ static class NumberedWeakReference extends WeakReference<Integer> {
+ // Add an integer to identify the weak reference object.
+ int number;
+
+ NumberedWeakReference(Integer referent, ReferenceQueue<Integer> q, int i) {
+ super(referent, q);
+ number = i;
+ }
+ }
+
+ final static boolean debug = System.getProperty("test.debug") != null;
+ final static int iterations = 1000;
+ final static int gc_trigger = 99;
+ static int[] a = new int[2 * iterations];
+ // Keep all weak references alive with the following array.
+ static NumberedWeakReference[] b = new NumberedWeakReference[iterations];
+
+ public static void main(String[] argv) throws Exception {
+ if (debug) {
+ System.out.println("Starting the test.");
+ }
+ // Raise thread priority to match the referenceHandler
+ // priority, so that they can race also on a uniprocessor.
+ raisePriority();
+
+ ReferenceQueue<Integer> refQueue = new ReferenceQueue<>();
+
+ // Our objective is to let the mutator enqueue
+ // a Reference object that may already be in the
+ // pending state because of having been identified
+ // as weakly reachable at a previous garbage collection.
+ // To this end, we create many Reference objects, each with a
+ // a unique integer object as its referant.
+ // We let the referents become eligible for collection,
+ // while racing with the garbage collector which may
+ // have pended some of these Reference objects.
+ // Finally we check that all of the Reference objects
+ // end up on the their queue. The test was originally
+ // submitted to show that such races could break the
+ // pending list and/or the reference queue, because of sharing
+ // the same link ("next") for maintaining both lists, thus
+ // losing some of the Reference objects on either queue.
+
+ Integer obj = new Integer(0);
+ NumberedWeakReference weaky = new NumberedWeakReference(obj, refQueue, 0);
+ for (int i = 1; i < iterations; i++) {
+ // Create a new object, dropping the onlY strong reference to
+ // the previous Integer object.
+ obj = new Integer(i);
+ // Trigger gc each gc_trigger iterations.
+ if ((i % gc_trigger) == 0) {
+ forceGc(0);
+ }
+ // Enqueue every other weaky.
+ if ((i % 2) == 0) {
+ weaky.enqueue();
+ }
+ // Remember the Reference objects, for testing later.
+ b[i - 1] = weaky;
+ // Get a new weaky for the Integer object just
+ // created, which may be explicitly enqueued in
+ // our next trip around the loop.
+ weaky = new NumberedWeakReference(obj, refQueue, i);
+ }
+
+ // Do a final collection to discover and process all
+ // Reference objects created above, allowing enough time
+ // for the ReferenceHandler thread to queue the References.
+ forceGc(100);
+ forceGc(100);
+
+ // Verify that all WeakReference objects ended up queued.
+ checkResult(refQueue, obj, iterations-1);
+ System.out.println("Test passed.");
+ }
+
+ private static void checkResult(ReferenceQueue<Integer> queue,
+ Integer obj,
+ int expected) {
+ if (debug) {
+ System.out.println("Reading the queue");
+ }
+
+ // Empty the queue and record numbers into a[];
+ NumberedWeakReference weakRead = (NumberedWeakReference) queue.poll();
+ int length = 0;
+ while (weakRead != null) {
+ a[length++] = weakRead.number;
+ weakRead = (NumberedWeakReference) queue.poll();
+ }
+ if (debug) {
+ System.out.println("Reference Queue had " + length + " elements");
+ }
+ // Use the last Reference object of those created above, so as to keep it "alive".
+ System.out.println("I must write " + obj + " to prevent compiler optimizations.");
+
+
+ // verify the queued references: all but the last Reference object
+ // should have been in the queue.
+ if (debug) {
+ System.out.println("Start of final check");
+ }
+
+ // Sort the first "length" elements in array "a[]".
+ sort(length);
+
+ boolean fail = (length != expected);
+ for (int i = 0; i < length; i++) {
+ if (a[i] != i) {
+ if (debug) {
+ System.out.println("a[" + i + "] is not " + i + " but " + a[i]);
+ }
+ fail = true;
+ }
+ }
+ if (fail) {
+ printMissingElements(length, expected);
+ throw new RuntimeException("TEST FAILED: only " + length
+ + " reference objects have been queued out of "
+ + expected);
+ }
+ }
+
+ private static void printMissingElements(int length, int expected) {
+ System.out.println("The following numbers were not found in the reference queue: ");
+ int missing = 0;
+ int element = 0;
+ for (int i = 0; i < length; i++) {
+ while ((a[i] != element) & (element < expected)) {
+ System.out.print(element + " ");
+ if (missing % 20 == 19) {
+ System.out.println(" ");
+ }
+ missing++;
+ element++;
+ }
+ element++;
+ }
+ System.out.print("\n");
+ }
+
+ private static void forceGc(long millis) throws InterruptedException {
+ Runtime.getRuntime().gc();
+ Thread.sleep(millis);
+ }
+
+ // Bubble sort the first "length" elements in array "a".
+ private static void sort(int length) {
+ int hold;
+ if (debug) {
+ System.out.println("Sorting. Length=" + length);
+ }
+ for (int pass = 1; pass < length; pass++) { // passes over the array
+ for (int i = 0; i < length - pass; i++) { // a single pass
+ if (a[i] > a[i + 1]) { // then swap
+ hold = a[i];
+ a[i] = a[i + 1];
+ a[i + 1] = hold;
+ }
+ } // End of i loop
+ } // End of pass loop
+ }
+
+ // Raise thread priority so as to increase the
+ // probability of the mutator succeeding in enqueueing
+ // an object that is still in the pending state.
+ // This is (probably) only required for a uniprocessor.
+ static void raisePriority() {
+ Thread tr = Thread.currentThread();
+ tr.setPriority(Thread.MAX_PRIORITY);
+ }
+} // End of class ReferenceEnqueuePending