From b9f1f77622c7a52b0eba39eb05c7841f5407f938 Mon Sep 17 00:00:00 2001
From: Eric Holk <eholk@mozilla.com>
Date: Fri, 12 Aug 2011 16:36:17 -0700
Subject: [PATCH] Fixed memory accounting and task stack creation bugs.
---
src/lib/task.rs | 3 ++
src/rt/memory_region.cpp | 77 +++++++++++++++++++++++++---------------
src/rt/memory_region.h | 5 ++-
src/rt/rust_builtin.cpp | 21 +++++++++--
src/rt/rust_task.cpp | 7 +++-
src/rt/rustrt.def.in | 1 +
src/test/stdtest/task.rs | 2 +-
7 files changed, 81 insertions(+), 35 deletions(-)
diff --git a/src/lib/task.rs b/src/lib/task.rs
index 6752e11595d..a99598fa573 100644
--- a/src/lib/task.rs
+++ b/src/lib/task.rs
@@ -21,6 +21,8 @@ native "rust" mod rustrt {
fn start_task(id : task_id);
fn get_task_trampoline() -> u32;
+ fn migrate_alloc(alloc : *u8, target : task_id);
+
fn leak[@T](thing : -T);
}
@@ -104,6 +106,7 @@ fn _spawn(thunk : fn() -> ()) -> task_id {
*env = raw_thunk.env;
*ra = rustrt::get_task_trampoline();
+ rustrt::migrate_alloc(cast(raw_thunk.env), id);
rustrt::start_task(id);
rustrt::leak(thunk);
diff --git a/src/rt/memory_region.cpp b/src/rt/memory_region.cpp
index 4ef3eb05ce1..7b38f65caaf 100644
--- a/src/rt/memory_region.cpp
+++ b/src/rt/memory_region.cpp
@@ -4,7 +4,7 @@
// NB: please do not commit code with this uncommented. It's
// hugely expensive and should only be used as a last resort.
//
-#define TRACK_ALLOCATIONS
+// #define TRACK_ALLOCATIONS
#define MAGIC 0xbadc0ffe
@@ -25,13 +25,13 @@ memory_region::memory_region(memory_region *parent) :
}
void memory_region::add_alloc() {
- _live_allocations++;
- //sync::increment(_live_allocations);
+ //_live_allocations++;
+ sync::increment(_live_allocations);
}
void memory_region::dec_alloc() {
- _live_allocations--;
- //sync::decrement(_live_allocations);
+ //_live_allocations--;
+ sync::decrement(_live_allocations);
}
void memory_region::free(void *mem) {
@@ -40,21 +40,10 @@ void memory_region::free(void *mem) {
if (_synchronized) { _lock.lock(); }
alloc_header *alloc = get_header(mem);
assert(alloc->magic == MAGIC);
-#ifdef TRACK_ALLOCATIONS
- if (_allocation_list[alloc->index] != alloc) {
- printf("free: ptr 0x%" PRIxPTR " (%s) is not in allocation_list\n",
- (uintptr_t) &alloc->data, alloc->tag);
- _srv->fatal("not in allocation_list", __FILE__, __LINE__, "");
- }
- else {
- // printf("freed index %d\n", index);
- _allocation_list[alloc->index] = NULL;
- }
-#endif
if (_live_allocations < 1) {
_srv->fatal("live_allocs < 1", __FILE__, __LINE__, "");
}
- dec_alloc();
+ release_alloc(mem);
_srv->free(alloc);
if (_synchronized) { _lock.unlock(); }
}
@@ -90,18 +79,13 @@ memory_region::realloc(void *mem, size_t size) {
void *
memory_region::malloc(size_t size, const char *tag, bool zero) {
if (_synchronized) { _lock.lock(); }
- add_alloc();
size_t old_size = size;
size += sizeof(alloc_header);
alloc_header *mem = (alloc_header *)_srv->malloc(size);
mem->magic = MAGIC;
mem->tag = tag;
-#ifdef TRACK_ALLOCATIONS
- mem->index = _allocation_list.append(mem);
- // printf("malloc: stored %p at index %d\n", mem, index);
-#endif
- // printf("malloc: ptr 0x%" PRIxPTR " region=%p\n",
- // (uintptr_t) mem, this);
+ mem->index = -1;
+ claim_alloc(mem->data);
if(zero) {
memset(mem->data, 0, old_size);
@@ -118,27 +102,32 @@ memory_region::calloc(size_t size, const char *tag) {
memory_region::~memory_region() {
if (_synchronized) { _lock.lock(); }
- if (_live_allocations == 0) {
+ if (_live_allocations == 0 && !_detailed_leaks) {
if (_synchronized) { _lock.unlock(); }
return;
}
char msg[128];
- snprintf(msg, sizeof(msg),
- "leaked memory in rust main loop (%" PRIuPTR " objects)",
- _live_allocations);
+ if(_live_allocations > 0) {
+ snprintf(msg, sizeof(msg),
+ "leaked memory in rust main loop (%d objects)",
+ _live_allocations);
+ }
#ifdef TRACK_ALLOCATIONS
if (_detailed_leaks) {
+ unsigned int leak_count = 0;
for (size_t i = 0; i < _allocation_list.size(); i++) {
if (_allocation_list[i] != NULL) {
alloc_header *header = (alloc_header*)_allocation_list[i];
printf("allocation (%s) 0x%" PRIxPTR " was not freed\n",
header->tag,
(uintptr_t) &header->data);
+ ++leak_count;
}
}
+ assert(leak_count == _live_allocations);
}
#endif
- if (!_hack_allow_leaks) {
+ if (!_hack_allow_leaks && _live_allocations > 0) {
_srv->fatal(msg, __FILE__, __LINE__,
"%d objects", _live_allocations);
}
@@ -150,6 +139,36 @@ memory_region::hack_allow_leaks() {
_hack_allow_leaks = true;
}
+void
+memory_region::release_alloc(void *mem) {
+ alloc_header *alloc = get_header(mem);
+ assert(alloc->magic == MAGIC);
+
+#ifdef TRACK_ALLOCATIONS
+ if (_allocation_list[alloc->index] != alloc) {
+ printf("free: ptr 0x%" PRIxPTR " (%s) is not in allocation_list\n",
+ (uintptr_t) &alloc->data, alloc->tag);
+ _srv->fatal("not in allocation_list", __FILE__, __LINE__, "");
+ }
+ else {
+ // printf("freed index %d\n", index);
+ _allocation_list[alloc->index] = NULL;
+ alloc->index = -1;
+ }
+#endif
+ dec_alloc();
+}
+
+void
+memory_region::claim_alloc(void *mem) {
+ alloc_header *alloc = get_header(mem);
+ assert(alloc->magic == MAGIC);
+#ifdef TRACK_ALLOCATIONS
+ alloc->index = _allocation_list.append(alloc);
+#endif
+ add_alloc();
+}
+
//
// Local Variables:
// mode: C++
diff --git a/src/rt/memory_region.h b/src/rt/memory_region.h
index c323e911039..26a21f45028 100644
--- a/src/rt/memory_region.h
+++ b/src/rt/memory_region.h
@@ -27,7 +27,7 @@ private:
rust_srv *_srv;
memory_region *_parent;
- size_t _live_allocations;
+ int _live_allocations;
array_list<alloc_header *> _allocation_list;
const bool _detailed_leaks;
const bool _synchronized;
@@ -48,6 +48,9 @@ public:
// to not kill the entire process, which the test runner needs. Please
// kill with prejudice once unwinding works.
void hack_allow_leaks();
+
+ void release_alloc(void *mem);
+ void claim_alloc(void *mem);
};
inline void *operator new(size_t size, memory_region ®ion,
diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index d330fbff590..77a4e8d2b09 100644
--- a/src/rt/rust_builtin.cpp
+++ b/src/rt/rust_builtin.cpp
@@ -289,7 +289,7 @@ task_yield(rust_task *task) {
extern "C" CDECL intptr_t
task_join(rust_task *task, rust_task_id tid) {
// If the other task is already dying, we don't have to wait for it.
- smart_ptr<rust_task> join_task = task->kernel->get_task_by_id(tid);
+ rust_task *join_task = task->kernel->get_task_by_id(tid);
// FIXME: find task exit status and return that.
if(!join_task) return 0;
join_task->lock.lock();
@@ -720,10 +720,11 @@ new_task(rust_task *task) {
extern "C" CDECL registers_t *
get_task_context(rust_task *task, rust_task_id id) {
- registers_t *regs = &task->kernel->get_task_by_id(id)->ctx.regs;
+ rust_task *target = task->kernel->get_task_by_id(id);
+ registers_t *regs = &target->ctx.regs;
// This next line is a little dangerous.. It means we can only safely call
// this when starting a task.
- regs->esp = task->rust_sp;
+ regs->esp = target->rust_sp;
return regs;
}
@@ -746,6 +747,20 @@ get_task_trampoline(rust_task *task) {
return &task_trampoline;
}
+extern "C" CDECL void
+migrate_alloc(rust_task *task, void *alloc, rust_task_id tid) {
+ if(!alloc) return;
+ rust_task *target = task->kernel->get_task_by_id(tid);
+ if(target) {
+ task->local_region.release_alloc(alloc);
+ target->local_region.claim_alloc(alloc);
+ }
+ else {
+ // We couldn't find the target. Maybe we should just free?
+ task->fail();
+ }
+}
+
extern "C" CDECL rust_chan *
clone_chan(rust_task *task, rust_chan *chan) {
return chan->clone(task);
diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index a03388a2a55..1a28e6fb947 100644
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -108,8 +108,13 @@ struct spawn_args {
};
extern "C" CDECL
-void task_exit(void *env, int rval, rust_task *task) {
+void task_exit(intptr_t *env, int rval, rust_task *task) {
LOG(task, task, "task exited with value %d", rval);
+ if(env) {
+ // free the environment.
+ I(task->sched, 1 == *env); // the ref count better be 1
+ task->free(env);
+ }
task->die();
task->lock.lock();
task->notify_tasks_waiting_to_join();
diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in
index 693db873b82..db31ef68067 100644
--- a/src/rt/rustrt.def.in
+++ b/src/rt/rustrt.def.in
@@ -41,6 +41,7 @@ ivec_reserve_shared
ivec_to_ptr
last_os_error
leak
+migrate_alloc
nano_time
new_chan
new_port
diff --git a/src/test/stdtest/task.rs b/src/test/stdtest/task.rs
index bde732ef6d9..1c48b6308fa 100644
--- a/src/test/stdtest/task.rs
+++ b/src/test/stdtest/task.rs
@@ -43,4 +43,4 @@ fn test_lib_spawn() {
fn test_lib_spawn2() {
fn foo(x : int) { assert(x == 42); }
task::_spawn(bind foo(42));
-}
\ No newline at end of file
+}