rt: Make linked failure less prone to deadlock

Still a mess.

5 files changed, 58 insertions, 31 deletions

diff --git a/src/rt/rust_port.cpp b/src/rt/rust_port.cpp
index 41884f8f60c..bbfa737adec 100644
--- a/src/rt/rust_port.cpp
+++ b/src/rt/rust_port.cpp
@@ -88,6 +88,11 @@ void rust_port::receive(void *dptr, uintptr_t *yield) {
     task->rendezvous_ptr = (uintptr_t*) dptr;
     task->block(this, "waiting for rendezvous data");
 
+    // Blocking the task might fail if the task has already been killed, but
+    // in the event of both failure and success the task needs to yield. On
+    // success, it yields and waits to be unblocked. On failure it yields and
+    // is then fails the task.
+
     *yield = true;
 }
 
diff --git a/src/rt/rust_port_selector.cpp b/src/rt/rust_port_selector.cpp
index 1e1b09a59c5..319b6fb579d 100644
--- a/src/rt/rust_port_selector.cpp
+++ b/src/rt/rust_port_selector.cpp
@@ -48,8 +48,14 @@ rust_port_selector::select(rust_task *task, rust_port **dptr,
         this->n_ports = n_ports;
         I(task->thread, task->rendezvous_ptr == NULL);
         task->rendezvous_ptr = (uintptr_t*)dptr;
-        *yield = true;
         task->block(this, "waiting for select rendezvous");
+
+	// Blocking the task might fail if the task has already been
+	// killed, but in the event of both failure and success the
+	// task needs to yield. On success, it yields and waits to be
+	// unblocked. On failure it yields and is then fails the task.
+
+        *yield = true;
     }
 
     for (size_t i = 0; i < locks_taken; i++) {
diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 71f4949888f..878bf099689 100644
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -246,6 +246,12 @@ void rust_task::start()
 bool
 rust_task::must_fail_from_being_killed() {
     scoped_lock with(kill_lock);
+    return must_fail_from_being_killed_unlocked();
+}
+
+bool
+rust_task::must_fail_from_being_killed_unlocked() {
+    I(thread, kill_lock.lock_held_by_current_thread());
     return killed && !reentered_rust_stack;
 }
 
@@ -253,6 +259,7 @@ rust_task::must_fail_from_being_killed() {
 void
 rust_task::yield(bool *killed) {
     if (must_fail_from_being_killed()) {
+        I(thread, !blocked());
         *killed = true;
     }
 
@@ -266,10 +273,11 @@ rust_task::yield(bool *killed) {
 
 void
 rust_task::kill() {
+    scoped_lock with(kill_lock);
+
     if (dead()) {
         // Task is already dead, can't kill what's already dead.
         fail_parent();
-        return;
     }
 
     // Note the distinction here: kill() is when you're in an upcall
@@ -277,15 +285,14 @@ rust_task::kill() {
     // If you want to fail yourself you do self->fail().
     LOG(this, task, "killing task %s @0x%" PRIxPTR, name, this);
     // When the task next goes to yield or resume it will fail
-    {
-        scoped_lock with(kill_lock);
-        killed = true;
-    }
+    killed = true;
     // Unblock the task so it can unwind.
-    unblock();
+
+    if (blocked()) {
+        wakeup(cond);
+    }
 
     LOG(this, task, "preparing to unwind task: 0x%" PRIxPTR, this);
-    // run_on_resume(rust_unwind_glue);
 }
 
 extern "C" CDECL
@@ -324,7 +331,6 @@ rust_task::fail_parent() {
              name, this, supervisor->name, supervisor);
         supervisor->kill();
     }
-    // FIXME: implement unwinding again.
     if (NULL == supervisor && propagate_failure)
         thread->fail();
 }
@@ -411,14 +417,23 @@ rust_task::set_state(rust_task_list *state,
     this->cond_name = cond_name;
 }
 
-void
+bool
 rust_task::block(rust_cond *on, const char* name) {
+    scoped_lock with(kill_lock);
+
+    if (must_fail_from_being_killed_unlocked()) {
+        // We're already going to die. Don't block. Tell the task to fail
+        return false;
+    }
+
     LOG(this, task, "Blocking on 0x%" PRIxPTR ", cond: 0x%" PRIxPTR,
                          (uintptr_t) on, (uintptr_t) cond);
     A(thread, cond == NULL, "Cannot block an already blocked task.");
     A(thread, on != NULL, "Cannot block on a NULL object.");
 
     transition(&thread->running_tasks, &thread->blocked_tasks, on, name);
+
+    return true;
 }
 
 void
@@ -436,15 +451,6 @@ rust_task::die() {
     transition(&thread->running_tasks, &thread->dead_tasks, NULL, "none");
 }
 
-void
-rust_task::unblock() {
-    if (blocked()) {
-        // FIXME: What if another thread unblocks the task between when
-        // we checked and here?
-        wakeup(cond);
-    }
-}
-
 rust_crate_cache *
 rust_task::get_crate_cache()
 {
diff --git a/src/rt/rust_task.h b/src/rt/rust_task.h
index 328b5c5ca9e..f822c109510 100644
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -140,6 +140,8 @@ private:
     void transition(rust_task_list *src, rust_task_list *dst,
                     rust_cond *cond, const char* cond_name);
 
+    bool must_fail_from_being_killed_unlocked();
+
     friend void task_start_wrapper(spawn_args *a);
     friend void cleanup_task(cleanup_args *a);
     friend void reset_stack_limit_on_c_stack(reset_args *a);
@@ -169,10 +171,9 @@ public:
     void set_state(rust_task_list *state,
                    rust_cond *cond, const char* cond_name);
 
-    void block(rust_cond *on, const char* name);
+    bool block(rust_cond *on, const char* name);
     void wakeup(rust_cond *from);
     void die();
-    void unblock();
 
     // Print a backtrace, if the "bt" logging option is on.
     void backtrace();
diff --git a/src/rt/rust_task_thread.cpp b/src/rt/rust_task_thread.cpp
index cf16f209a4e..e108a21f3a9 100644
--- a/src/rt/rust_task_thread.cpp
+++ b/src/rt/rust_task_thread.cpp
@@ -2,6 +2,7 @@
 #include <stdarg.h>
 #include <cassert>
 #include <pthread.h>
+#include <vector>
 #include "rust_internal.h"
 #include "rust_util.h"
 #include "globals.h"
@@ -95,19 +96,27 @@ rust_task_thread::fail() {
 
 void
 rust_task_thread::kill_all_tasks() {
-    I(this, !lock.lock_held_by_current_thread());
-    scoped_lock with(lock);
+    std::vector<rust_task*> all_tasks;
 
-    for (size_t i = 0; i < running_tasks.length(); i++) {
-        // We don't want the failure of these tasks to propagate back
-        // to the kernel again since we're already failing everything
-        running_tasks[i]->unsupervise();
-        running_tasks[i]->kill();
+    {
+        scoped_lock with(lock);
+
+        for (size_t i = 0; i < running_tasks.length(); i++) {
+            all_tasks.push_back(running_tasks[i]);
+        }
+
+        for (size_t i = 0; i < blocked_tasks.length(); i++) {
+            all_tasks.push_back(blocked_tasks[i]);
+        }
     }
 
-    for (size_t i = 0; i < blocked_tasks.length(); i++) {
-        blocked_tasks[i]->unsupervise();
-        blocked_tasks[i]->kill();
+    while (!all_tasks.empty()) {
+        rust_task *task = all_tasks.back();
+        all_tasks.pop_back();
+        // We don't want the failure of these tasks to propagate back
+        // to the kernel again since we're already failing everything
+        task->unsupervise();
+        task->kill();
     }
 }