Revert linked failure (lifecycle lock)

This reverts commit 74f4b8d.

Author: bblum

src/rt/rust_sched_loop.cpp

@@ -208,7 +208,7 @@ rust_sched_loop::run_single_turn() {
             return sched_loop_state_block;
         }
 
-        scheduled_task->assert_is_running();
+        assert(scheduled_task->running());
 
         DLOG(this, task,
              "activating task %s 0x%" PRIxPTR

src/rt/rust_task.cpp

@@ -114,7 +114,7 @@ cleanup_task(cleanup_args *args) {
     rust_task *task = a->task;
 
     {
-        scoped_lock with(task->lifecycle_lock);
+        scoped_lock with(task->kill_lock);
         if (task->killed && !threw_exception) {
             LOG(task, task, "Task killed during termination");
             threw_exception = true;
@@ -230,25 +230,21 @@ void rust_task::start()
 
 bool
 rust_task::must_fail_from_being_killed() {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(kill_lock);
     return must_fail_from_being_killed_unlocked();
 }
 
 bool
 rust_task::must_fail_from_being_killed_unlocked() {
-    lifecycle_lock.must_have_lock();
+    kill_lock.must_have_lock();
     return killed && !reentered_rust_stack && disallow_kill == 0;
 }
 
 // Only run this on the rust stack
 void
 rust_task::yield(bool *killed) {
-    // FIXME (#2787): clean this up
     if (must_fail_from_being_killed()) {
-        {
-            scoped_lock with(lifecycle_lock);
-            assert(!(state == task_state_blocked));
-        }
+        assert(!blocked());
         *killed = true;
     }
 
@@ -262,7 +258,7 @@ rust_task::yield(bool *killed) {
 
 void
 rust_task::kill() {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(kill_lock);
 
     // XXX: bblum: kill/kill race
 
@@ -274,9 +270,8 @@ rust_task::kill() {
     killed = true;
     // Unblock the task so it can unwind.
 
-    if (state == task_state_blocked &&
-        must_fail_from_being_killed_unlocked()) {
-        wakeup_locked(cond);
+    if (blocked() && must_fail_from_being_killed_unlocked()) {
+        wakeup(cond);
     }
 
     LOG(this, task, "preparing to unwind task: 0x%" PRIxPTR, this);
@@ -340,21 +335,34 @@ rust_task::get_frame_glue_fns(uintptr_t fp) {
     return *((frame_glue_fns**) fp);
 }
 
-void rust_task::assert_is_running()
+bool
+rust_task::running()
+{
+    scoped_lock with(state_lock);
+    return state == task_state_running;
+}
+
+bool
+rust_task::blocked()
 {
-    scoped_lock with(lifecycle_lock);
-    assert(state == task_state_running);
+    scoped_lock with(state_lock);
+    return state == task_state_blocked;
 }
 
-// FIXME (#2851, #2787): This is only used by rust_port/rust_port selector,
-// and is inherently racy. Get rid of it.
 bool
 rust_task::blocked_on(rust_cond *on)
 {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(state_lock);
     return cond == on;
 }
 
+bool
+rust_task::dead()
+{
+    scoped_lock with(state_lock);
+    return state == task_state_dead;
+}
+
 void *
 rust_task::malloc(size_t sz, const char *tag, type_desc *td)
 {
@@ -376,28 +384,28 @@ rust_task::free(void *p)
 void
 rust_task::transition(rust_task_state src, rust_task_state dst,
                       rust_cond *cond, const char* cond_name) {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(state_lock);
     transition_locked(src, dst, cond, cond_name);
 }
 
 void rust_task::transition_locked(rust_task_state src, rust_task_state dst,
                                   rust_cond *cond, const char* cond_name) {
-    lifecycle_lock.must_have_lock();
+    state_lock.must_have_lock();
     sched_loop->transition(this, src, dst, cond, cond_name);
 }
 
 void
 rust_task::set_state(rust_task_state state,
                      rust_cond *cond, const char* cond_name) {
-    lifecycle_lock.must_have_lock();
+    state_lock.must_have_lock();
     this->state = state;
     this->cond = cond;
     this->cond_name = cond_name;
 }
 
 bool
 rust_task::block(rust_cond *on, const char* name) {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(kill_lock);
     return block_locked(on, name);
 }
 
@@ -420,7 +428,7 @@ rust_task::block_locked(rust_cond *on, const char* name) {
 
 void
 rust_task::wakeup(rust_cond *from) {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(state_lock);
     wakeup_locked(from);
 }
 
@@ -668,27 +676,26 @@ rust_task::on_rust_stack() {
 
 void
 rust_task::inhibit_kill() {
-    scoped_lock with(lifecycle_lock);
-    // FIXME (#1868) Check here if we have to die
+    scoped_lock with(kill_lock);
     disallow_kill++;
 }
 
 void
 rust_task::allow_kill() {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(kill_lock);
     assert(disallow_kill > 0 && "Illegal allow_kill(): already killable!");
     disallow_kill--;
 }
 
 void *
 rust_task::wait_event(bool *killed) {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(state_lock);
 
     if(!event_reject) {
         block_locked(&event_cond, "waiting on event");
-        lifecycle_lock.unlock();
+        state_lock.unlock();
         yield(killed);
-        lifecycle_lock.lock();
+        state_lock.lock();
     }
 
     event_reject = false;
@@ -697,7 +704,7 @@ rust_task::wait_event(bool *killed) {
 
 void
 rust_task::signal_event(void *event) {
-    scoped_lock with(lifecycle_lock);
+    scoped_lock with(state_lock);
 
     this->event = event;
     event_reject = true;

src/rt/rust_task.h

@@ -170,8 +170,7 @@ rust_task : public kernel_owned<rust_task>
 private:
 
     // Protects state, cond, cond_name
-    // Protects the killed flag, disallow_kill flag, reentered_rust_stack
-    lock_and_signal lifecycle_lock;
+    lock_and_signal state_lock;
     rust_task_state state;
     rust_cond *cond;
     const char *cond_name;
@@ -180,6 +179,8 @@ rust_task : public kernel_owned<rust_task>
     rust_cond event_cond;
     void *event;
 
+    // Protects the killed flag, disallow_kill flag, reentered_rust_stack
+    lock_and_signal kill_lock;
     // Indicates that the task was killed and needs to unwind
     bool killed;
     // Indicates that we've called back into Rust from C
@@ -242,8 +243,10 @@ rust_task : public kernel_owned<rust_task>
                rust_opaque_box *env,
                void *args);
     void start();
-    void assert_is_running();
-    bool blocked_on(rust_cond *cond); // FIXME (#2851) Get rid of this.
+    bool running();
+    bool blocked();
+    bool blocked_on(rust_cond *cond);
+    bool dead();
 
     void *malloc(size_t sz, const char *tag, type_desc *td=0);
     void *realloc(void *data, size_t sz);
@@ -435,8 +438,7 @@ rust_task::call_on_rust_stack(void *args, void *fn_ptr) {
 
     bool had_reentered_rust_stack = reentered_rust_stack;
     {
-        // FIXME (#2787) This must be racy. Figure it out.
-        scoped_lock with(lifecycle_lock);
+        scoped_lock with(kill_lock);
         reentered_rust_stack = true;
     }
 
@@ -451,7 +453,7 @@ rust_task::call_on_rust_stack(void *args, void *fn_ptr) {
 
     next_c_sp = prev_c_sp;
     {
-        scoped_lock with(lifecycle_lock);
+        scoped_lock with(kill_lock);
         reentered_rust_stack = had_reentered_rust_stack;
     }