Fixed memory accounting and task stack creation bugs.

Author: Eric Holk

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);

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++

src/rt/memory_region.h

@@ -27,7 +27,7 @@ class memory_region {
 
     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 @@ class memory_region {
     // 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 &region,

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);

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();

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

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));
-}
+}