Proper locking with blocked_on()/wakeup() in rust_port. Closes #2787. Closes #1923.

Author: bblum

src/rt/rust_port.cpp

@@ -26,9 +26,11 @@ void rust_port::deref() {
     scoped_lock with(ref_lock);
     ref_count--;
     if (!ref_count) {
+        // The port owner is waiting for the port to be detached (if it
+        // hasn't already been killed)
+        scoped_lock with(task->lifecycle_lock);
         if (task->blocked_on(&detach_cond)) {
-            // The port owner is waiting for the port to be detached
-            task->wakeup(&detach_cond);
+            task->wakeup_inner(&detach_cond);
         }
     }
 }
@@ -64,12 +66,15 @@ void rust_port::send(void *sptr) {
         assert(!buffer.is_empty() &&
                "rust_chan::transmit with nothing to send.");
 
-        if (task->blocked_on(this)) {
-            KLOG(kernel, comm, "dequeued in rendezvous_ptr");
-            buffer.dequeue(task->rendezvous_ptr);
-            task->rendezvous_ptr = 0;
-            task->wakeup(this);
-            did_rendezvous = true;
+        {
+            scoped_lock with(task->lifecycle_lock);
+            if (task->blocked_on(this)) {
+                KLOG(kernel, comm, "dequeued in rendezvous_ptr");
+                buffer.dequeue(task->rendezvous_ptr);
+                task->rendezvous_ptr = 0;
+                task->wakeup_inner(this);
+                did_rendezvous = true;
+            }
         }
     }
 
@@ -78,11 +83,8 @@ void rust_port::send(void *sptr) {
         // it may be waiting on a group of ports
 
         rust_port_selector *port_selector = task->get_port_selector();
-        // This check is not definitive. The port selector will take a lock
-        // and check again whether the task is still blocked.
-        if (task->blocked_on(port_selector)) {
-            port_selector->msg_sent_on(this);
-        }
+        // The port selector will check if the task is blocked, not us.
+        port_selector->msg_sent_on(this);
     }
 }
 

src/rt/rust_port_selector.cpp

@@ -75,7 +75,7 @@ rust_port_selector::msg_sent_on(rust_port *port) {
 
     // Prevent two ports from trying to wake up the task
     // simultaneously
-    scoped_lock with(rendezvous_lock);
+    scoped_lock with(task->lifecycle_lock);
 
     if (task->blocked_on(this)) {
         for (size_t i = 0; i < n_ports; i++) {
@@ -85,7 +85,7 @@ rust_port_selector::msg_sent_on(rust_port *port) {
                 n_ports = 0;
                 *task->rendezvous_ptr = (uintptr_t) port;
                 task->rendezvous_ptr = NULL;
-                task->wakeup(this);
+                task->wakeup_inner(this);
                 return;
             }
         }

src/rt/rust_port_selector.h

@@ -9,7 +9,6 @@ class rust_port_selector : public rust_cond {
  private:
     rust_port **ports;
     size_t n_ports;
-    lock_and_signal rendezvous_lock;
 
  public:
     rust_port_selector();

src/rt/rust_task.cpp

@@ -243,7 +243,7 @@ rust_task::must_fail_from_being_killed_inner() {
 // Only run this on the rust stack
 void
 rust_task::yield(bool *killed) {
-    // FIXME (#2787): clean this up
+    // FIXME (#2875): clean this up
     if (must_fail_from_being_killed()) {
         {
             scoped_lock with(lifecycle_lock);
@@ -346,12 +346,11 @@ void rust_task::assert_is_running()
     assert(state == task_state_running);
 }
 
-// FIXME (#2851, #2787): This is only used by rust_port/rust_port selector,
-// and is inherently racy. Get rid of it.
+// FIXME (#2851) Remove this code when rust_port goes away?
 bool
 rust_task::blocked_on(rust_cond *on)
 {
-    scoped_lock with(lifecycle_lock);
+    lifecycle_lock.must_have_lock();
     return cond == on;
 }
 

src/rt/rust_task.h

@@ -226,8 +226,11 @@ rust_task : public kernel_owned<rust_task>
                                char const *file,
                                size_t line);
 
+    friend class rust_port;
+    friend class rust_port_selector;
     bool block_inner(rust_cond *on, const char* name);
     void wakeup_inner(rust_cond *from);
+    bool blocked_on(rust_cond *cond);
 
 public:
 
@@ -243,7 +246,6 @@ rust_task : public kernel_owned<rust_task>
                void *args);
     void start();
     void assert_is_running();
-    bool blocked_on(rust_cond *cond); // FIXME (#2851) Get rid of this.
 
     void *malloc(size_t sz, const char *tag, type_desc *td=0);
     void *realloc(void *data, size_t sz);
@@ -435,7 +437,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.
+        // FIXME (#2875) This must be racy. Figure it out.
         scoped_lock with(lifecycle_lock);
         reentered_rust_stack = true;
     }