[Truffle] fix deadlock in Mutex, ConditionVariable implementation

jruby · Jul 22, 2016 · b59ea63 · b59ea63
1 parent 7789a89
commit b59ea63
Showing 1 changed file with 31 additions and 21 deletions.
diff --git a/truffle/src/main/java/org/jruby/truffle/core/mutex/ConditionVariableNodes.java b/truffle/src/main/java/org/jruby/truffle/core/mutex/ConditionVariableNodes.java
@@ -80,30 +80,40 @@ public DynamicObject wait(DynamicObject conditionVariable, DynamicObject mutex,
         }
 
         @TruffleBoundary
-        private void doWait(final long durationInMillis, ReentrantLock lock, DynamicObject thread, Object condition) {
+        private void doWait(
+                final long durationInMillis,
+                ReentrantLock lock,
+                DynamicObject thread,
+                Object condition) {
+
             final long start = System.currentTimeMillis();
 
-            // First lock the condition so we only release the Mutex when we are in wait() and ready to be notified
-            synchronized (condition) {
-                MutexOperations.unlock(lock, thread, this);
-                try {
-                    getContext().getThreadManager().runUntilResult(this, new BlockingAction<Boolean>() {
-                        @Override
-                        public Boolean block() throws InterruptedException {
-                            long now = System.currentTimeMillis();
-                            long slept = now - start;
-
-                            if (slept >= durationInMillis) {
-                                return SUCCESS;
-                            }
-
-                            condition.wait(durationInMillis - slept);
-                            return SUCCESS;
-                        }
-                    });
-                } finally {
-                    MutexOperations.lock(lock, thread, this);
+            try {
+                // First lock the condition so we only release the Mutex when we are in wait()
+                // and ready to be notified
+                synchronized (condition) {
+                    MutexOperations.unlock(lock, thread, this);
+                    getContext().getThreadManager().
+                            runUntilResult(this, new BlockingAction<Boolean>() {
+                                @Override
+                                public Boolean block() throws InterruptedException {
+                                    long now = System.currentTimeMillis();
+                                    long slept = now - start;
+
+                                    if (slept >= durationInMillis) {
+                                        return SUCCESS;
+                                    }
+
+                                    condition.wait(durationInMillis - slept);
+                                    return SUCCESS;
+                                }
+                            });
                 }
+            } finally {
+                // Has to lock again *after* condition lock is released, otherwise it would be
+                // locked in reverse order condition > mutex opposed to normal locking order
+                // mutex > condition, which would lead to deadlock.
+                MutexOperations.lock(lock, thread, this);
             }
         }
     }