8073706: Livelock in CompiledFunction.getValidOptimisticInvocation
authorattila
Wed, 11 Mar 2015 17:52:23 +0100
changeset 29414 4c635c98c0b4
parent 29413 153d198cfdc3
child 29415 7f36aa566efe
8073706: Livelock in CompiledFunction.getValidOptimisticInvocation Reviewed-by: hannesw, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Wed Mar 11 17:47:28 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CompiledFunction.java	Wed Mar 11 17:52:23 2015 +0100
@@ -27,6 +27,7 @@
 import static jdk.nashorn.internal.lookup.Lookup.MH;
 import static jdk.nashorn.internal.runtime.UnwarrantedOptimismException.INVALID_PROGRAM_POINT;
 import static jdk.nashorn.internal.runtime.UnwarrantedOptimismException.isValid;
+
 import java.lang.invoke.CallSite;
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
@@ -594,7 +595,7 @@
      * switchpoint has been invalidated by a {@code RewriteException} triggered on another thread for this function.
      * This is not a problem, though, as these switch points are always used to produce call sites that fall back to
      * relinking when they are invalidated, and in this case the execution will end up here again. What this method
-     * basically does is minimize such busy-loop relinking while the function is being recompiled on a different thread.
+     * basically does is reduce such busy-loop relinking while the function is being recompiled on a different thread.
      * @param invocationSupplier the supplier that constructs the actual invocation method handle; should use the
      * {@code CompiledFunction} method itself in some capacity.
      * @return a tuple object containing the method handle as created by the supplier and an optimistic assumptions
@@ -602,20 +603,27 @@
      * function can't be further deoptimized).
      */
     private synchronized HandleAndAssumptions getValidOptimisticInvocation(final Supplier<MethodHandle> invocationSupplier) {
-        for(;;) {
+        for(int i = 0; i < 2; ++i) {
             final MethodHandle handle = invocationSupplier.get();
             final SwitchPoint assumptions = canBeDeoptimized() ? optimismInfo.optimisticAssumptions : null;
-            if(assumptions != null && assumptions.hasBeenInvalidated()) {
+            if(i == 0 && assumptions != null && assumptions.hasBeenInvalidated()) {
                 // We can be in a situation where one thread is in the middle of a deoptimizing compilation when we hit
                 // this and thus, it has invalidated the old switch point, but hasn't created the new one yet. Note that
                 // the behavior of invalidating the old switch point before recompilation, and only creating the new one
-                // after recompilation is by design. If we didn't wait here for the recompilation to complete, we would
-                // be busy looping through the fallback path of the invalidated switch point, relinking the call site
-                // again with the same invalidated switch point, invoking the fallback, etc. stealing CPU cycles from
-                // the recompilation task we're dependent on. This can still happen if the switch point gets invalidated
-                // after we grabbed it here, in which case we'll indeed do one busy relink immediately.
+                // after recompilation is by design. If we didn't wait here, we would be busy looping through the
+                // fallback path of the invalidated switch point, relinking the call site again with the same
+                // invalidated switch point, invoking the fallback, etc. stealing CPU cycles from the recompilation
+                // task we're dependent on. This can still happen if the switch point gets invalidated after we grabbed
+                // it here, in which case we'll indeed do one busy relink immediately.
+                // On the other hand, in order to avoid a rare livelock, we aren't doing an infinite loop, and we
+                // aren't wait()-ing indefinitely. We'll do at most one, at most 1000ms long wait after which we'll
+                // return the current handle even if it's invalidated (and which'll then trigger one loop through the
+                // relink mechanism). We therefore strike a balance between busy looping and a livelock risk by making
+                // sure that there's at most one iteration of busy loop per second. It is theoretically possible to
+                // correctly implement this without ever risking a livelock, but using this heuristic we eliminate the
+                // chance of the livelock, while still maintaining a good enough busy-looping prevention.
                 try {
-                    wait();
+                    wait(1000L);
                 } catch (final InterruptedException e) {
                     // Intentionally ignored. There's nothing meaningful we can do if we're interrupted
                 }
@@ -623,6 +631,7 @@
                 return new HandleAndAssumptions(handle, assumptions);
             }
         }
+        throw new AssertionError(); // never reached
     }
 
     private static void relinkComposableInvoker(final CallSite cs, final CompiledFunction inv, final boolean constructor) {