Change yield() and wait_event() to be MUST_CHECK and return the killed flag. (closes #2875)

4 files changed, 32 insertions, 19 deletions

diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index d2c09e81434..f80e2e822c0 100644
--- a/src/rt/rust_builtin.cpp
+++ b/src/rt/rust_builtin.cpp
@@ -722,9 +722,9 @@ rust_port_id_send(rust_port_id target_port_id, void *sptr) {
 
 // This is called by an intrinsic on the Rust stack and must run
 // entirely in the red zone. Do not call on the C stack.
-extern "C" CDECL void
+extern "C" CDECL MUST_CHECK bool
 rust_task_yield(rust_task *task, bool *killed) {
-    task->yield(killed);
+    return task->yield();
 }
 
 extern "C" CDECL void
@@ -920,8 +920,8 @@ rust_wait_cond_lock(rust_cond_lock *lock) {
     lock->waiting = task;
     task->block(lock, "waiting for signal");
     lock->lock.unlock();
-    bool killed = false;
-    task->yield(&killed);
+    bool killed = task->yield();
+    assert(!killed && "unimplemented");
     lock->lock.lock();
 }
 
@@ -960,12 +960,12 @@ task_clear_event_reject(rust_task *task) {
 
 // Waits on an event, returning the pointer to the event that unblocked this
 // task.
-extern "C" void *
-task_wait_event(rust_task *task, bool *killed) {
-    // FIXME #2890: we should assert that the passed in task is the currently
+extern "C" MUST_CHECK bool
+task_wait_event(rust_task *task, void **result) {
+    // Maybe (if not too slow) assert that the passed in task is the currently
     // running task. We wouldn't want to wait some other task.
 
-    return task->wait_event(killed);
+    return task->wait_event(result);
 }
 
 extern "C" void
diff --git a/src/rt/rust_globals.h b/src/rt/rust_globals.h
index 7056f7f4a27..2d69edebd0e 100644
--- a/src/rt/rust_globals.h
+++ b/src/rt/rust_globals.h
@@ -85,6 +85,8 @@ extern "C" int check_claims;
         }                                                           \
     }
 
+#define MUST_CHECK __attribute__((warn_unused_result))
+
 #define PTR "0x%" PRIxPTR
 
 // This accounts for logging buffers.
diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 014e8e7948d..e7a18e90461 100644
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -242,26 +242,30 @@ void rust_task_yield_fail(rust_task *task) {
 }
 
 // Only run this on the rust stack
-void
-rust_task::yield(bool *killed) {
+MUST_CHECK bool rust_task::yield() {
+    bool killed = false;
+
     if (disallow_yield > 0) {
         call_on_c_stack(this, (void *)rust_task_yield_fail);
     }
-    // FIXME (#2875): clean this up
+
+    // This check is largely superfluous; it's the one after the context swap
+    // that really matters. This one allows us to assert a useful invariant.
     if (must_fail_from_being_killed()) {
         {
             scoped_lock with(lifecycle_lock);
             assert(!(state == task_state_blocked));
         }
-        *killed = true;
+        killed = true;
     }
 
     // Return to the scheduler.
     ctx.next->swap(ctx);
 
     if (must_fail_from_being_killed()) {
-        *killed = true;
+        killed = true;
     }
+    return killed;
 }
 
 void
@@ -687,19 +691,24 @@ void rust_task::allow_yield() {
     disallow_yield--;
 }
 
-void *
-rust_task::wait_event(bool *killed) {
+MUST_CHECK bool rust_task::wait_event(void **result) {
+    bool killed = false;
     scoped_lock with(lifecycle_lock);
 
     if(!event_reject) {
         block_inner(&event_cond, "waiting on event");
         lifecycle_lock.unlock();
-        yield(killed);
+        killed = yield();
         lifecycle_lock.lock();
+    } else if (must_fail_from_being_killed_inner()) {
+        // If the deschedule was rejected, yield won't do our killed check for
+        // us. For thoroughness, do it here. FIXME (#524)
+        killed = true;
     }
 
     event_reject = false;
-    return event;
+    *result = event;
+    return killed;
 }
 
 void
diff --git a/src/rt/rust_task.h b/src/rt/rust_task.h
index 3c0fcc4c574..8df119565f0 100644
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -259,7 +259,8 @@ public:
     void backtrace();
 
     // Yields control to the scheduler. Called from the Rust stack
-    void yield(bool *killed);
+    // Returns TRUE if the task was killed and needs to fail.
+    MUST_CHECK bool yield();
 
     // Fail this task (assuming caller-on-stack is different task).
     void kill();
@@ -311,7 +312,8 @@ public:
         this->event_reject = false;
     }
 
-    void *wait_event(bool *killed);
+    // Returns TRUE if the task was killed and needs to fail.
+    MUST_CHECK bool wait_event(void **result);
     void signal_event(void *event);
 
     void cleanup_after_turn();