From 3d9023fa4ddc8dbb5d9be0e4e4ef5c284c6b077a Mon Sep 17 00:00:00 2001
From: Brian Anderson <banderson@mozilla.com>
Date: Fri, 11 Nov 2011 15:34:35 -0800
Subject: [PATCH] rt: Take the task lock when dropping port refcounts

Sucks, but otherwise there are races when one task drops the refcount to zero
followed by another bumping it again
---
 src/rt/rust_builtin.cpp           |  4 +++-
 src/rt/rust_task.cpp              | 33 +++++++++++++------------------
 src/rt/rust_task.h                |  2 --
 src/test/run-pass/task-comm-15.rs |  7 +++----
 4 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index 30b915f4e96..e8109ea7a04 100644
--- a/src/rt/rust_builtin.cpp
+++ b/src/rt/rust_builtin.cpp
@@ -471,6 +471,7 @@ extern "C" CDECL void
 del_port(rust_port *port) {
     rust_task *task = rust_scheduler::get_task();
     LOG(task, comm, "del_port(0x%" PRIxPTR ")", (uintptr_t) port);
+    scoped_lock with(task->lock);
     port->deref();
 }
 
@@ -487,11 +488,12 @@ chan_id_send(type_desc *t, rust_task_id target_task_id,
     rust_task *target_task = task->kernel->get_task_by_id(target_task_id);
     if(target_task) {
         rust_port *port = target_task->get_port_by_id(target_port_id);
-        target_task->deref();
         if(port) {
             port->send(sptr);
+            scoped_lock with(target_task->lock);
             port->deref();
         }
+        target_task->deref();
     }
 }
 
diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 5164f6ac181..e5d43e7b308 100644
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -127,15 +127,20 @@ rust_task::~rust_task()
          name, (uintptr_t)this, ref_count);
 
     if(user.notify_enabled) {
-        rust_port *target =
-            get_port_by_chan_handle(&user.notify_chan);
-        if(target) {
-            task_notification msg;
-            msg.id = user.id;
-            msg.result = failed ? tr_failure : tr_success;
+        rust_task *target_task = kernel->get_task_by_id(user.notify_chan.task);
+        if (target_task) {
+            rust_port *target_port =
+                target_task->get_port_by_id(user.notify_chan.port);
+            if(target_port) {
+                task_notification msg;
+                msg.id = user.id;
+                msg.result = failed ? tr_failure : tr_success;
 
-            target->send(&msg);
-            target->deref();
+                target_port->send(&msg);
+                scoped_lock with(target_task->lock);
+                target_port->deref();
+            }
+            target_task->deref();
         }
     }
 
@@ -553,8 +558,7 @@ rust_port_id rust_task::register_port(rust_port *port) {
 }
 
 void rust_task::release_port(rust_port_id id) {
-    I(sched, !lock.lock_held_by_current_thread());
-    scoped_lock with(lock);
+    I(sched, lock.lock_held_by_current_thread());
     port_table.remove(id);
 }
 
@@ -569,15 +573,6 @@ rust_port *rust_task::get_port_by_id(rust_port_id id) {
     return port;
 }
 
-rust_port *rust_task::get_port_by_chan_handle(chan_handle *handle) {
-    rust_task *target_task = kernel->get_task_by_id(handle->task);
-    if(target_task) {
-        rust_port *port = target_task->get_port_by_id(handle->port);
-        target_task->deref();
-        return port;
-    }
-    return NULL;
-}
 
 // Temporary routine to allow boxes on one task's shared heap to be reparented
 // to another.
diff --git a/src/rt/rust_task.h b/src/rt/rust_task.h
index 3bdc900250d..2d8afa619cd 100644
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -209,8 +209,6 @@ rust_task : public kernel_owned<rust_task>, rust_cond
     // not at all safe.
     intptr_t get_ref_count() const { return ref_count; }
 
-    rust_port *get_port_by_chan_handle(chan_handle *handle);
-
     // FIXME: These functions only exist to get the tasking system off the
     // ground. We should never be migrating shared boxes between tasks.
     const type_desc *release_alloc(void *alloc);
diff --git a/src/test/run-pass/task-comm-15.rs b/src/test/run-pass/task-comm-15.rs
index 2876441673c..69b11519d28 100644
--- a/src/test/run-pass/task-comm-15.rs
+++ b/src/test/run-pass/task-comm-15.rs
@@ -1,11 +1,10 @@
-// xfail-test
 // xfail-win32
 use std;
 import std::comm;
 import std::task;
 
-fn start(c: comm::chan<int>, n: int) {
-    let i: int = n;
+fn start(&&args: (comm::chan<int>, int)) {
+    let (c, i) = args;
 
     while i > 0 { comm::send(c, 0); i = i - 1; }
 }
@@ -16,6 +15,6 @@ fn main() {
     // is likely to terminate before the child completes, so from
     // the child's point of view the receiver may die. We should
     // drop messages on the floor in this case, and not crash!
-    let child = task::spawn(bind start(comm::chan(p), 10));
+    let child = task::spawn((comm::chan(p), 10), start);
     let c = comm::recv(p);
 }