8175797: (ref) Reference::enqueue method should clear referent before enqueuing

Reviewed-by: alanb, kbarrett, mr

jdk/src/java.base/share/classes/java/lang/ref/FinalReference.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -33,4 +33,9 @@ class FinalReference<T> extends Reference<T> {
     public FinalReference(T referent, ReferenceQueue<? super T> q) {
         super(referent, q);
     }
+
+    @Override
+    public boolean enqueue() {
+        throw new InternalError("should never reach here");
+    }
 }

jdk/src/java.base/share/classes/java/lang/ref/Reference.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2017, 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
@@ -140,15 +140,24 @@ public abstract class Reference<T> {
         }
     }
 
-    /* Atomically get and clear (set to null) the VM's pending list.
+    /*
+     * system property to disable clearing before enqueuing.
+     */
+    private static final boolean disableClearBeforeEnqueue
+        = Boolean.getBoolean("jdk.lang.ref.disableClearBeforeEnqueue");
+
+    /*
+     * Atomically get and clear (set to null) the VM's pending list.
      */
     private static native Reference<Object> getAndClearReferencePendingList();
 
-    /* Test whether the VM's pending list contains any entries.
+    /*
+     * Test whether the VM's pending list contains any entries.
      */
     private static native boolean hasReferencePendingList();
 
-    /* Wait until the VM's pending list may be non-null.
+    /*
+     * Wait until the VM's pending list may be non-null.
      */
     private static native void waitForReferencePendingList();
 
@@ -261,7 +270,6 @@ public abstract class Reference<T> {
         this.referent = null;
     }
 
-
     /* -- Queue operations -- */
 
     /**
@@ -278,8 +286,8 @@ public abstract class Reference<T> {
     }
 
     /**
-     * Adds this reference object to the queue with which it is registered,
+     * Clears this reference object and adds it to the queue with which
-     * if any.
+     * it is registered, if any.
      *
      * <p> This method is invoked only by Java code; when the garbage collector
      * enqueues references it does so directly, without invoking this method.
@@ -289,10 +297,11 @@ public abstract class Reference<T> {
      *           it was not registered with a queue when it was created
      */
     public boolean enqueue() {
+        if (!disableClearBeforeEnqueue)
+            this.referent = null;
         return this.queue.enqueue(this);
     }
 
-
     /* -- Constructors -- */
 
     Reference(T referent) {
@@ -419,5 +428,4 @@ public abstract class Reference<T> {
         // HotSpot needs to retain the ref and not GC it before a call to this
         // method
     }
-
 }

jdk/test/java/lang/ref/ReferenceEnqueue.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2017, 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
@@ -22,17 +22,23 @@
  */
 
 /* @test
- * @bug 4268317 8132306
+ * @bug 4268317 8132306 8175797
  * @summary Test if Reference.enqueue() works properly with GC
+ * @run main ReferenceEnqueue
+ * @run main/othervm -Djdk.lang.ref.disableClearAndEnqueue=true ReferenceEnqueue
  */
 
 import java.lang.ref.*;
+import java.util.ArrayList;
+import java.util.List;
 
 public class ReferenceEnqueue {
 
     public static void main(String args[]) throws Exception {
-        for (int i=0; i < 5; i++)
+        for (int i=0; i < 5; i++) {
             new WeakRef().run();
+            new ExplicitEnqueue().run();
+        }
         System.out.println("Test passed.");
     }
 
@@ -76,4 +82,50 @@ public class ReferenceEnqueue {
             }
         }
     }
+
+    static class ExplicitEnqueue {
+        final ReferenceQueue<Object> queue = new ReferenceQueue<>();
+        final List<Reference<Object>> refs = new ArrayList<>();
+        final int iterations = 1000;
+        final boolean disableClearAndEnqueue =
+            Boolean.parseBoolean("jdk.lang.ref.disableClearAndEnqueue");
+
+        ExplicitEnqueue() {
+            this.refs.add(new SoftReference<>(new Object(), queue));
+            this.refs.add(new WeakReference<>(new Object(), queue));
+            this.refs.add(new PhantomReference<>(new Object(), queue));
+        }
+
+        void run() throws InterruptedException {
+            for (Reference<Object> ref : refs) {
+                if (ref.enqueue() == false) {
+                    throw new RuntimeException("Error: enqueue failed");
+                }
+                if (disableClearAndEnqueue && ref.get() == null) {
+                    throw new RuntimeException("Error: clearing should be disabled");
+                }
+                if (!disableClearAndEnqueue && ref.get() != null) {
+                    throw new RuntimeException("Error: referent must be cleared");
+                }
+            }
+
+            System.gc();
+            for (int i = 0; refs.size() > 0 && i < iterations; i++) {
+                Reference<Object> ref = (Reference<Object>)queue.poll();
+                if (ref == null) {
+                    System.gc();
+                    Thread.sleep(100);
+                    continue;
+                }
+
+                if (refs.remove(ref) == false) {
+                    throw new RuntimeException("Error: unknown reference " + ref);
+                }
+            }
+
+            if (!refs.isEmpty()) {
+                throw new RuntimeException("Error: not all references are removed");
+            }
+        }
+    }
 }