From 08e72c5fda2cea5eab9bf35eebadd32f73eace05 Mon Sep 17 00:00:00 2001 From: Rob Arnold Date: Wed, 29 Jun 2011 20:54:03 -0700 Subject: [PATCH 1/4] Move channel cloning logic into a method on rust_chan. This will allow us to more easily clone channels from native code. --- src/rt/rust_chan.cpp | 19 +++++++++++++++++++ src/rt/rust_chan.h | 2 ++ src/rt/rust_upcall.cpp | 16 +--------------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/rt/rust_chan.cpp b/src/rt/rust_chan.cpp index a947a1c93e8e2..a1c71b049ea20 100644 --- a/src/rt/rust_chan.cpp +++ b/src/rt/rust_chan.cpp @@ -97,6 +97,25 @@ void rust_chan::send(void *sptr) { return; } + +rust_chan *rust_chan::clone(maybe_proxy *target) { + size_t unit_sz = buffer.unit_sz; + maybe_proxy *port = this->port; + rust_task *target_task = NULL; + if (target->is_proxy() == false) { + port = this->port; + target_task = target->referent(); + } else { + rust_handle *handle = + task->sched->kernel->get_port_handle(port->as_referent()); + maybe_proxy *proxy = new rust_proxy (handle); + LOG(task, mem, "new proxy: " PTR, proxy); + port = proxy; + target_task = target->as_proxy()->handle()->referent(); + } + return new (target_task) rust_chan(target_task, port, unit_sz); +} + // // Local Variables: // mode: C++ diff --git a/src/rt/rust_chan.h b/src/rt/rust_chan.h index a6e4d3d6b43c9..603936d5b771c 100644 --- a/src/rt/rust_chan.h +++ b/src/rt/rust_chan.h @@ -19,6 +19,8 @@ class rust_chan : public rc_base, bool is_associated(); void send(void *sptr); + + rust_chan *clone(maybe_proxy *target); }; // diff --git a/src/rt/rust_upcall.cpp b/src/rt/rust_upcall.cpp index 4f7dccc465247..2d3f8c7e1391b 100644 --- a/src/rt/rust_upcall.cpp +++ b/src/rt/rust_upcall.cpp @@ -166,21 +166,7 @@ upcall_clone_chan(rust_task *task, maybe_proxy *target, rust_chan *chan) { LOG_UPCALL_ENTRY(task); scoped_lock with(task->kernel->scheduler_lock); - size_t unit_sz = chan->buffer.unit_sz; - maybe_proxy *port = chan->port; - rust_task *target_task = NULL; - if (target->is_proxy() == false) { - port = chan->port; - target_task = target->referent(); - } else { - rust_handle *handle = - task->sched->kernel->get_port_handle(port->as_referent()); - maybe_proxy *proxy = new rust_proxy (handle); - LOG(task, mem, "new proxy: " PTR, proxy); - port = proxy; - target_task = target->as_proxy()->handle()->referent(); - } - return new (target_task) rust_chan(target_task, port, unit_sz); + return chan->clone(target); } extern "C" CDECL void From 568a1faf1f07d20d639d0bb19215fa3c5b71e674 Mon Sep 17 00:00:00 2001 From: Rob Arnold Date: Thu, 30 Jun 2011 08:29:35 -0700 Subject: [PATCH 2/4] Move the channel destroy code into rust_chan. This lets native code more easily destroy channels since directly deleting a channel is not always the right way to destroy it. --- src/rt/rust_chan.cpp | 35 +++++++++++++++++++++++++++++++++++ src/rt/rust_chan.h | 3 +++ src/rt/rust_upcall.cpp | 30 +----------------------------- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/rt/rust_chan.cpp b/src/rt/rust_chan.cpp index a1c71b049ea20..3065a55e58260 100644 --- a/src/rt/rust_chan.cpp +++ b/src/rt/rust_chan.cpp @@ -116,6 +116,41 @@ rust_chan *rust_chan::clone(maybe_proxy *target) { return new (target_task) rust_chan(target_task, port, unit_sz); } +/** + * Cannot Yield: If the task were to unwind, the dropped ref would still + * appear to be live, causing modify-after-free errors. + */ +void rust_chan::destroy() { + A(task->sched, ref_count == 0, + "Channel's ref count should be zero."); + + if (is_associated()) { + if (port->is_proxy()) { + // Here is a good place to delete the port proxy we allocated + // in upcall_clone_chan. + rust_proxy *proxy = port->as_proxy(); + disassociate(); + delete proxy; + } else { + // We're trying to delete a channel that another task may be + // reading from. We have two options: + // + // 1. We can flush the channel by blocking in upcall_flush_chan() + // and resuming only when the channel is flushed. The problem + // here is that we can get ourselves in a deadlock if the + // parent task tries to join us. + // + // 2. We can leave the channel in a "dormnat" state by not freeing + // it and letting the receiver task delete it for us instead. + if (buffer.is_empty() == false) { + return; + } + disassociate(); + } + } + delete this; +} + // // Local Variables: // mode: C++ diff --git a/src/rt/rust_chan.h b/src/rt/rust_chan.h index 603936d5b771c..a327dbf0c95d4 100644 --- a/src/rt/rust_chan.h +++ b/src/rt/rust_chan.h @@ -21,6 +21,9 @@ class rust_chan : public rc_base, void send(void *sptr); rust_chan *clone(maybe_proxy *target); + + // Called whenever the channel's ref count drops to zero. + void destroy(); }; // diff --git a/src/rt/rust_upcall.cpp b/src/rt/rust_upcall.cpp index 2d3f8c7e1391b..05220f026065c 100644 --- a/src/rt/rust_upcall.cpp +++ b/src/rt/rust_upcall.cpp @@ -126,35 +126,7 @@ void upcall_del_chan(rust_task *task, rust_chan *chan) { scoped_lock with(task->kernel->scheduler_lock); LOG(task, comm, "upcall del_chan(0x%" PRIxPTR ")", (uintptr_t) chan); - - A(task->sched, chan->ref_count == 0, - "Channel's ref count should be zero."); - - if (chan->is_associated()) { - if (chan->port->is_proxy()) { - // Here is a good place to delete the port proxy we allocated - // in upcall_clone_chan. - rust_proxy *proxy = chan->port->as_proxy(); - chan->disassociate(); - delete proxy; - } else { - // We're trying to delete a channel that another task may be - // reading from. We have two options: - // - // 1. We can flush the channel by blocking in upcall_flush_chan() - // and resuming only when the channel is flushed. The problem - // here is that we can get ourselves in a deadlock if the - // parent task tries to join us. - // - // 2. We can leave the channel in a "dormant" state by not freeing - // it and letting the receiver task delete it for us instead. - if (chan->buffer.is_empty() == false) { - return; - } - chan->disassociate(); - } - } - delete chan; + chan->destroy(); } /** From 52aa80976fc73cef33b1a3b325ba4e0de46fa066 Mon Sep 17 00:00:00 2001 From: Rob Arnold Date: Thu, 30 Jun 2011 19:24:45 -0700 Subject: [PATCH 3/4] Add macro for refcounting runtime structures. The macro with the extra dtor parameter is intended for structures like rust_chan which may not necessarily delete themselves when the ref count becomes 0. This functionality will be used in an upcoming changeset. --- src/rt/rust_internal.h | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/rt/rust_internal.h b/src/rt/rust_internal.h index d5c4f459f1c7e..9d7d714064fd7 100644 --- a/src/rt/rust_internal.h +++ b/src/rt/rust_internal.h @@ -97,19 +97,18 @@ static intptr_t const CONST_REFCOUNT = 0x7badface; static size_t const BUF_BYTES = 2048; // Every reference counted object should derive from this base class. +// Or use this macro. The macro is preferred as the base class will be +// disappearing. -template struct rc_base { - intptr_t ref_count; - - void ref() { - ++ref_count; - } +#define RUST_REFCOUNTED(T) \ + RUST_REFCOUNTED_WITH_DTOR(T, delete (T*)this) +#define RUST_REFCOUNTED_WITH_DTOR(T, dtor) \ + intptr_t ref_count; \ + void ref() { ++ref_count; } \ + void deref() { if (--ref_count == 0) { dtor; } } - void deref() { - if (--ref_count == 0) { - delete (T*)this; - } - } +template struct rc_base { + RUST_REFCOUNTED(T) rc_base(); ~rc_base(); From 4d09b4d01cac05629c0dbff7ee73715adaed6dbc Mon Sep 17 00:00:00 2001 From: Rob Arnold Date: Thu, 30 Jun 2011 19:27:35 -0700 Subject: [PATCH 4/4] Sync rust_chan's deref() method with rustc's code. If the channel is associated with a port then the destructor will assert. Additionally, destruction of the object is not always appropriate. This brings the deref() method into sync with the behavior of generated rust code which only invokes destroy() once the reference count goes to 0. --- src/rt/rust_chan.cpp | 1 + src/rt/rust_chan.h | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rt/rust_chan.cpp b/src/rt/rust_chan.cpp index 3065a55e58260..78301e3d879dd 100644 --- a/src/rt/rust_chan.cpp +++ b/src/rt/rust_chan.cpp @@ -7,6 +7,7 @@ rust_chan::rust_chan(rust_task *task, maybe_proxy *port, size_t unit_sz) : + ref_count(1), task(task), port(port), buffer(task, unit_sz) { diff --git a/src/rt/rust_chan.h b/src/rt/rust_chan.h index a327dbf0c95d4..4172ca5f09d86 100644 --- a/src/rt/rust_chan.h +++ b/src/rt/rust_chan.h @@ -1,10 +1,10 @@ #ifndef RUST_CHAN_H #define RUST_CHAN_H -class rust_chan : public rc_base, - public task_owned, +class rust_chan : public task_owned, public rust_cond { public: + RUST_REFCOUNTED_WITH_DTOR(rust_chan, destroy()) rust_chan(rust_task *task, maybe_proxy *port, size_t unit_sz); ~rust_chan();