8156500: Move Reference pending list into VM to prevent deadlocks
authorkbarrett
Tue, 30 Aug 2016 23:46:02 -0400
changeset 40954 72ca7f63532a
parent 40953 3b16ca3af29f
child 40955 c0e02245ff93
child 40956 4326ea45a1c0
8156500: Move Reference pending list into VM to prevent deadlocks Summary: Move reference pending list and locking into VM Reviewed-by: coleenp, dholmes, dcubed, mchung, plevart Contributed-by: kim.barrett@oracle.com, per.liden@oracle.com
jdk/make/mapfiles/libjava/mapfile-vers
jdk/src/java.base/share/classes/java/lang/ref/Reference.java
jdk/src/java.base/share/classes/java/nio/Bits.java
jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangRefAccess.java
jdk/src/java.base/share/native/include/jvm.h
jdk/src/java.base/share/native/libjava/Reference.c
jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java
--- a/jdk/make/mapfiles/libjava/mapfile-vers	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/make/mapfiles/libjava/mapfile-vers	Tue Aug 30 23:46:02 2016 -0400
@@ -176,6 +176,9 @@
 		Java_java_lang_ProcessHandleImpl_00024Info_info0;
 		Java_java_lang_ProcessImpl_init;
 		Java_java_lang_ProcessImpl_forkAndExec;
+		Java_java_lang_ref_Reference_getAndClearReferencePendingList;
+		Java_java_lang_ref_Reference_hasReferencePendingList;
+		Java_java_lang_ref_Reference_waitForReferencePendingList;
 		Java_java_lang_reflect_Array_get;
 		Java_java_lang_reflect_Array_getBoolean;
 		Java_java_lang_reflect_Array_getByte;
--- a/jdk/src/java.base/share/classes/java/lang/ref/Reference.java	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/src/java.base/share/classes/java/lang/ref/Reference.java	Tue Aug 30 23:46:02 2016 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2016, 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
@@ -110,22 +110,6 @@
     private transient Reference<T> discovered;  /* used by VM */
 
 
-    /* Object used to synchronize with the garbage collector.  The collector
-     * must acquire this lock at the beginning of each collection cycle.  It is
-     * therefore critical that any code holding this lock complete as quickly
-     * as possible, allocate no new objects, and avoid calling user code.
-     */
-    private static class Lock { }
-    private static Lock lock = new Lock();
-
-
-    /* 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. The
-     * list uses the discovered field to link its elements.
-     */
-    private static Reference<Object> pending = null;
-
     /* High-priority thread to enqueue pending References
      */
     private static class ReferenceHandler extends Thread {
@@ -139,10 +123,9 @@
         }
 
         static {
-            // pre-load and initialize InterruptedException and Cleaner classes
-            // so that we don't get into trouble later in the run loop if there's
-            // memory shortage while loading/initializing them lazily.
-            ensureClassInitialized(InterruptedException.class);
+            // pre-load and initialize Cleaner class so that we don't
+            // get into trouble later in the run loop if there's
+            // memory shortage while loading/initializing it lazily.
             ensureClassInitialized(Cleaner.class);
         }
 
@@ -152,72 +135,80 @@
 
         public void run() {
             while (true) {
-                tryHandlePending(true);
+                processPendingReferences();
             }
         }
     }
 
-    /**
-     * Try handle pending {@link Reference} if there is one.<p>
-     * Return {@code true} as a hint that there might be another
-     * {@link Reference} pending or {@code false} when there are no more pending
-     * {@link Reference}s at the moment and the program can do some other
-     * useful work instead of looping.
-     *
-     * @param waitForNotify if {@code true} and there was no pending
-     *                      {@link Reference}, wait until notified from VM
-     *                      or interrupted; if {@code false}, return immediately
-     *                      when there is no pending {@link Reference}.
-     * @return {@code true} if there was a {@link Reference} pending and it
-     *         was processed, or we waited for notification and either got it
-     *         or thread was interrupted before being notified;
-     *         {@code false} otherwise.
+    /* 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.
+     */
+    private static native boolean hasReferencePendingList();
+
+    /* Wait until the VM's pending list may be non-null.
      */
-    static boolean tryHandlePending(boolean waitForNotify) {
-        Reference<Object> r;
-        Cleaner c;
-        try {
-            synchronized (lock) {
-                if (pending != null) {
-                    r = pending;
-                    // 'instanceof' might throw OutOfMemoryError sometimes
-                    // so do this before un-linking 'r' from the 'pending' chain...
-                    c = r instanceof Cleaner ? (Cleaner) r : null;
-                    // unlink 'r' from 'pending' chain
-                    pending = r.discovered;
-                    r.discovered = null;
-                } else {
-                    // The waiting on the lock may cause an OutOfMemoryError
-                    // because it may try to allocate exception objects.
-                    if (waitForNotify) {
-                        lock.wait();
-                    }
-                    // retry if waited
-                    return waitForNotify;
+    private static native void waitForReferencePendingList();
+
+    private static final Object processPendingLock = new Object();
+    private static boolean processPendingActive = false;
+
+    private static void processPendingReferences() {
+        // Only the singleton reference processing thread calls
+        // waitForReferencePendingList() and getAndClearReferencePendingList().
+        // These are separate operations to avoid a race with other threads
+        // that are calling waitForReferenceProcessing().
+        waitForReferencePendingList();
+        Reference<Object> pendingList;
+        synchronized (processPendingLock) {
+            pendingList = getAndClearReferencePendingList();
+            processPendingActive = true;
+        }
+        while (pendingList != null) {
+            Reference<Object> ref = pendingList;
+            pendingList = ref.discovered;
+            ref.discovered = null;
+
+            if (ref instanceof Cleaner) {
+                ((Cleaner)ref).clean();
+                // Notify any waiters that progress has been made.
+                // This improves latency for nio.Bits waiters, which
+                // are the only important ones.
+                synchronized (processPendingLock) {
+                    processPendingLock.notifyAll();
                 }
+            } else {
+                ReferenceQueue<? super Object> q = ref.queue;
+                if (q != ReferenceQueue.NULL) q.enqueue(ref);
             }
-        } catch (OutOfMemoryError x) {
-            // Give other threads CPU time so they hopefully drop some live references
-            // and GC reclaims some space.
-            // Also prevent CPU intensive spinning in case 'r instanceof Cleaner' above
-            // persistently throws OOME for some time...
-            Thread.yield();
-            // retry
-            return true;
-        } catch (InterruptedException x) {
-            // retry
-            return true;
+        }
+        // Notify any waiters of completion of current round.
+        synchronized (processPendingLock) {
+            processPendingActive = false;
+            processPendingLock.notifyAll();
         }
+    }
 
-        // Fast path for cleaners
-        if (c != null) {
-            c.clean();
-            return true;
+    // Wait for progress in reference processing.
+    //
+    // Returns true after waiting (for notification from the reference
+    // processing thread) if either (1) the VM has any pending
+    // references, or (2) the reference processing thread is
+    // processing references. Otherwise, returns false immediately.
+    private static boolean waitForReferenceProcessing()
+        throws InterruptedException
+    {
+        synchronized (processPendingLock) {
+            if (processPendingActive || hasReferencePendingList()) {
+                // Wait for progress, not necessarily completion.
+                processPendingLock.wait();
+                return true;
+            } else {
+                return false;
+            }
         }
-
-        ReferenceQueue<? super Object> q = r.queue;
-        if (q != ReferenceQueue.NULL) q.enqueue(r);
-        return true;
     }
 
     static {
@@ -236,8 +227,10 @@
         // provide access in SharedSecrets
         SharedSecrets.setJavaLangRefAccess(new JavaLangRefAccess() {
             @Override
-            public boolean tryHandlePendingReference() {
-                return tryHandlePending(false);
+            public boolean waitForReferenceProcessing()
+                throws InterruptedException
+            {
+                return Reference.waitForReferenceProcessing();
             }
         });
     }
--- a/jdk/src/java.base/share/classes/java/nio/Bits.java	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/src/java.base/share/classes/java/nio/Bits.java	Tue Aug 30 23:46:02 2016 -0400
@@ -131,23 +131,38 @@
         }
 
         final JavaLangRefAccess jlra = SharedSecrets.getJavaLangRefAccess();
-
-        // retry while helping enqueue pending Reference objects
-        // which includes executing pending Cleaner(s) which includes
-        // Cleaner(s) that free direct buffer memory
-        while (jlra.tryHandlePendingReference()) {
-            if (tryReserveMemory(size, cap)) {
-                return;
-            }
-        }
-
-        // trigger VM's Reference processing
-        System.gc();
-
-        // a retry loop with exponential back-off delays
-        // (this gives VM some time to do it's job)
         boolean interrupted = false;
         try {
+
+            // Retry allocation until success or there are no more
+            // references (including Cleaners that might free direct
+            // buffer memory) to process and allocation still fails.
+            boolean refprocActive;
+            do {
+                try {
+                    refprocActive = jlra.waitForReferenceProcessing();
+                } catch (InterruptedException e) {
+                    // Defer interrupts and keep trying.
+                    interrupted = true;
+                    refprocActive = true;
+                }
+                if (tryReserveMemory(size, cap)) {
+                    return;
+                }
+            } while (refprocActive);
+
+            // trigger VM's Reference processing
+            System.gc();
+
+            // A retry loop with exponential back-off delays.
+            // Sometimes it would suffice to give up once reference
+            // processing is complete.  But if there are many threads
+            // competing for memory, this gives more opportunities for
+            // any given thread to make progress.  In particular, this
+            // seems to be enough for a stress test like
+            // DirectBufferAllocTest to (usually) succeed, while
+            // without it that test likely fails.  Since failure here
+            // ends in OOME, there's no need to hurry.
             long sleepTime = 1;
             int sleeps = 0;
             while (true) {
@@ -157,14 +172,14 @@
                 if (sleeps >= MAX_SLEEPS) {
                     break;
                 }
-                if (!jlra.tryHandlePendingReference()) {
-                    try {
+                try {
+                    if (!jlra.waitForReferenceProcessing()) {
                         Thread.sleep(sleepTime);
                         sleepTime <<= 1;
                         sleeps++;
-                    } catch (InterruptedException e) {
-                        interrupted = true;
                     }
+                } catch (InterruptedException e) {
+                    interrupted = true;
                 }
             }
 
--- a/jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangRefAccess.java	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/src/java.base/share/classes/jdk/internal/misc/JavaLangRefAccess.java	Tue Aug 30 23:46:02 2016 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2016, 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
@@ -28,12 +28,12 @@
 public interface JavaLangRefAccess {
 
     /**
-     * Help ReferenceHandler thread process next pending
-     * {@link java.lang.ref.Reference}
+     * Wait for progress in {@link java.lang.ref.Reference}
+     * processing.  If there aren't any pending {@link
+     * java.lang.ref.Reference}s, return immediately.
      *
-     * @return {@code true} if there was a pending reference and it
-     *         was enqueue-ed or {@code false} if there was no
-     *         pending reference
+     * @return {@code true} if there were any pending
+     * {@link java.lang.ref.Reference}s, {@code false} otherwise.
      */
-    boolean tryHandlePendingReference();
+    boolean waitForReferenceProcessing() throws InterruptedException;
 }
--- a/jdk/src/java.base/share/native/include/jvm.h	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/src/java.base/share/native/include/jvm.h	Tue Aug 30 23:46:02 2016 -0400
@@ -282,6 +282,18 @@
 JVM_GetSystemPackages(JNIEnv *env);
 
 /*
+ * java.lang.ref.Reference
+ */
+JNIEXPORT jobject JNICALL
+JVM_GetAndClearReferencePendingList(JNIEnv *env);
+
+JNIEXPORT jboolean JNICALL
+JVM_HasReferencePendingList(JNIEnv *env);
+
+JNIEXPORT void JNICALL
+JVM_WaitForReferencePendingList(JNIEnv *env);
+
+/*
  * java.io.ObjectInputStream
  */
 JNIEXPORT jobject JNICALL
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/java.base/share/native/libjava/Reference.c	Tue Aug 30 23:46:02 2016 -0400
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2016, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+#include "jvm.h"
+#include "java_lang_ref_Reference.h"
+
+JNIEXPORT jobject JNICALL
+Java_java_lang_ref_Reference_getAndClearReferencePendingList(JNIEnv *env, jclass ignore)
+{
+    return JVM_GetAndClearReferencePendingList(env);
+}
+
+JNIEXPORT jboolean JNICALL
+Java_java_lang_ref_Reference_hasReferencePendingList(JNIEnv *env, jclass ignore)
+{
+    return JVM_HasReferencePendingList(env);
+}
+
+JNIEXPORT void JNICALL
+Java_java_lang_ref_Reference_waitForReferencePendingList(JNIEnv *env, jclass ignore)
+{
+    JVM_WaitForReferencePendingList(env);
+}
--- a/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java	Mon Aug 29 11:59:52 2016 +0300
+++ b/jdk/test/sun/security/provider/FileInputStreamPool/FileInputStreamPoolTest.java	Tue Aug 30 23:46:02 2016 -0400
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2016, 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
@@ -69,7 +69,7 @@
         // make JVM process References
         System.gc();
         // help ReferenceHandler thread enqueue References
-        while (TestProxy.Reference_tryHandlePending(false)) {}
+        while (TestProxy.Reference_waitForReferenceProcessing()) { }
         // help run Finalizers
         System.runFinalization();
     }
@@ -103,11 +103,11 @@
     /**
      * A proxy for (package)private static methods:
      *   sun.security.provider.FileInputStreamPool.getInputStream
-     *   java.lang.ref.Reference.tryHandlePending
+     *   java.lang.ref.Reference.waitForReferenceProcessing
      */
     static class TestProxy {
         private static final Method getInputStreamMethod;
-        private static final Method tryHandlePendingMethod;
+        private static final Method waitForReferenceProcessingMethod;
 
         static {
             try {
@@ -118,9 +118,9 @@
                         "getInputStream", File.class);
                 getInputStreamMethod.setAccessible(true);
 
-                tryHandlePendingMethod = Reference.class.getDeclaredMethod(
-                    "tryHandlePending", boolean.class);
-                tryHandlePendingMethod.setAccessible(true);
+                waitForReferenceProcessingMethod =
+                    Reference.class.getDeclaredMethod("waitForReferenceProcessing");
+                waitForReferenceProcessingMethod.setAccessible(true);
             } catch (Exception e) {
                 throw new Error(e);
             }
@@ -146,13 +146,14 @@
             }
         }
 
-        static boolean Reference_tryHandlePending(boolean waitForNotify) {
+        static boolean Reference_waitForReferenceProcessing() {
             try {
-                return (boolean) tryHandlePendingMethod
-                    .invoke(null, waitForNotify);
+                return (boolean) waitForReferenceProcessingMethod.invoke(null);
             } catch (InvocationTargetException e) {
                 Throwable te = e.getTargetException();
-                if (te instanceof RuntimeException) {
+                if (te instanceof InterruptedException) {
+                    return true;
+                } else if (te instanceof RuntimeException) {
                     throw (RuntimeException) te;
                 } else if (te instanceof Error) {
                     throw (Error) te;