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
This commit is contained in:
Brian Anderson 2011-11-11 15:34:35 -08:00
parent 07771ec25b
commit 3d9023fa4d
4 changed files with 20 additions and 26 deletions

View File

@ -471,6 +471,7 @@ extern "C" CDECL void
del_port(rust_port *port) { del_port(rust_port *port) {
rust_task *task = rust_scheduler::get_task(); rust_task *task = rust_scheduler::get_task();
LOG(task, comm, "del_port(0x%" PRIxPTR ")", (uintptr_t) port); LOG(task, comm, "del_port(0x%" PRIxPTR ")", (uintptr_t) port);
scoped_lock with(task->lock);
port->deref(); 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); rust_task *target_task = task->kernel->get_task_by_id(target_task_id);
if(target_task) { if(target_task) {
rust_port *port = target_task->get_port_by_id(target_port_id); rust_port *port = target_task->get_port_by_id(target_port_id);
target_task->deref();
if(port) { if(port) {
port->send(sptr); port->send(sptr);
scoped_lock with(target_task->lock);
port->deref(); port->deref();
} }
target_task->deref();
} }
} }

View File

@ -127,15 +127,20 @@ rust_task::~rust_task()
name, (uintptr_t)this, ref_count); name, (uintptr_t)this, ref_count);
if(user.notify_enabled) { if(user.notify_enabled) {
rust_port *target = rust_task *target_task = kernel->get_task_by_id(user.notify_chan.task);
get_port_by_chan_handle(&user.notify_chan); if (target_task) {
if(target) { rust_port *target_port =
task_notification msg; target_task->get_port_by_id(user.notify_chan.port);
msg.id = user.id; if(target_port) {
msg.result = failed ? tr_failure : tr_success; task_notification msg;
msg.id = user.id;
msg.result = failed ? tr_failure : tr_success;
target->send(&msg); target_port->send(&msg);
target->deref(); 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) { void rust_task::release_port(rust_port_id id) {
I(sched, !lock.lock_held_by_current_thread()); I(sched, lock.lock_held_by_current_thread());
scoped_lock with(lock);
port_table.remove(id); port_table.remove(id);
} }
@ -569,15 +573,6 @@ rust_port *rust_task::get_port_by_id(rust_port_id id) {
return port; 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 // Temporary routine to allow boxes on one task's shared heap to be reparented
// to another. // to another.

View File

@ -209,8 +209,6 @@ rust_task : public kernel_owned<rust_task>, rust_cond
// not at all safe. // not at all safe.
intptr_t get_ref_count() const { return ref_count; } 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 // FIXME: These functions only exist to get the tasking system off the
// ground. We should never be migrating shared boxes between tasks. // ground. We should never be migrating shared boxes between tasks.
const type_desc *release_alloc(void *alloc); const type_desc *release_alloc(void *alloc);

View File

@ -1,11 +1,10 @@
// xfail-test
// xfail-win32 // xfail-win32
use std; use std;
import std::comm; import std::comm;
import std::task; import std::task;
fn start(c: comm::chan<int>, n: int) { fn start(&&args: (comm::chan<int>, int)) {
let i: int = n; let (c, i) = args;
while i > 0 { comm::send(c, 0); i = i - 1; } 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 // is likely to terminate before the child completes, so from
// the child's point of view the receiver may die. We should // the child's point of view the receiver may die. We should
// drop messages on the floor in this case, and not crash! // 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); let c = comm::recv(p);
} }