8197812: (ref) Data race in Finalizer
authormartin
Thu, 15 Feb 2018 19:35:17 -0800
changeset 48903 d9fce53461a1
parent 48902 dfa46cfe5634
child 48904 3ae9318001f8
8197812: (ref) Data race in Finalizer Reviewed-by: plevart, mchung
src/java.base/share/classes/java/lang/ref/Finalizer.java
--- a/src/java.base/share/classes/java/lang/ref/Finalizer.java	Thu Feb 15 14:44:14 2018 +0000
+++ b/src/java.base/share/classes/java/lang/ref/Finalizer.java	Thu Feb 15 19:35:17 2018 -0800
@@ -36,18 +36,18 @@
                                                           class */
 
     private static ReferenceQueue<Object> queue = new ReferenceQueue<>();
+
+    /** Head of doubly linked list of Finalizers awaiting finalization. */
     private static Finalizer unfinalized = null;
+
+    /** Lock guarding access to unfinalized list. */
     private static final Object lock = new Object();
 
-    private Finalizer
-        next = null,
-        prev = null;
+    private Finalizer next, prev;
 
-    private boolean hasBeenFinalized() {
-        return (next == this);
-    }
-
-    private void add() {
+    private Finalizer(Object finalizee) {
+        super(finalizee, queue);
+        // push onto unfinalized
         synchronized (lock) {
             if (unfinalized != null) {
                 this.next = unfinalized;
@@ -57,31 +57,6 @@
         }
     }
 
-    private void remove() {
-        synchronized (lock) {
-            if (unfinalized == this) {
-                if (this.next != null) {
-                    unfinalized = this.next;
-                } else {
-                    unfinalized = this.prev;
-                }
-            }
-            if (this.next != null) {
-                this.next.prev = this.prev;
-            }
-            if (this.prev != null) {
-                this.prev.next = this.next;
-            }
-            this.next = this;   /* Indicates that this has been finalized */
-            this.prev = this;
-        }
-    }
-
-    private Finalizer(Object finalizee) {
-        super(finalizee, queue);
-        add();
-    }
-
     static ReferenceQueue<Object> getQueue() {
         return queue;
     }
@@ -91,11 +66,24 @@
         new Finalizer(finalizee);
     }
 
+    private void deregisterAndRunFinalizer(JavaLangAccess jla) {
+        synchronized (lock) {
+            if (this.next == this)      // already finalized
+                return;
+            // unlink from unfinalized
+            if (unfinalized == this)
+                unfinalized = this.next;
+            else
+                this.prev.next = this.next;
+            if (this.next != null)
+                this.next.prev = this.prev;
+            this.prev = null;
+            this.next = this;           // mark as finalized
+        }
+        runFinalizer(jla);
+    }
+
     private void runFinalizer(JavaLangAccess jla) {
-        synchronized (this) {
-            if (hasBeenFinalized()) return;
-            remove();
-        }
         try {
             Object finalizee = this.get();
             if (finalizee != null && !(finalizee instanceof java.lang.Enum)) {
@@ -155,11 +143,8 @@
                     return;
                 final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
                 running = true;
-                for (;;) {
-                    Finalizer f = (Finalizer)queue.poll();
-                    if (f == null) break;
-                    f.runFinalizer(jla);
-                }
+                for (Finalizer f; (f = (Finalizer)queue.poll()) != null; )
+                    f.deregisterAndRunFinalizer(jla);
             }
         });
     }
@@ -179,11 +164,15 @@
                 final JavaLangAccess jla = SharedSecrets.getJavaLangAccess();
                 running = true;
                 for (;;) {
+                    // "pollFirst" from unfinalized
                     Finalizer f;
                     synchronized (lock) {
                         f = unfinalized;
                         if (f == null) break;
                         unfinalized = f.next;
+                        if (unfinalized != null)
+                            unfinalized.prev = null;
+                        f.next = f; // mark as finalized
                     }
                     f.runFinalizer(jla);
                 }}});
@@ -214,7 +203,7 @@
             for (;;) {
                 try {
                     Finalizer f = (Finalizer)queue.remove();
-                    f.runFinalizer(jla);
+                    f.deregisterAndRunFinalizer(jla);
                 } catch (InterruptedException x) {
                     // ignore and continue
                 }