rt: Release big stacks immediately after use to avoid holding on to them through...

This avoids the following pathological scenario that makes threadring OOM:

1) task calls C using fast_ffi, borrowing a big stack from the scheduler.
2) task returns from C and places the big stack on the task-local stack segment list
3) task calls further Rust functions that require growing the stack, and for this reuses the big stack
4) task yields, failing to return the big stack to the scheduler.
5) repeat 500+ times and OOM

Conflicts:
	src/rt/rust_task.cpp

diff --git a/src/rt/rust_sched_loop.h b/src/rt/rust_sched_loop.h
index a099c5e0c74955c82409dd79c121521db4dfed0b..e0101c46fb9903cf3509d7eab88fa52832cbf65f 100644 (file)
--- a/src/rt/rust_sched_loop.h
+++ b/src/rt/rust_sched_loop.h
@@ -211,12 +211,13 @@ rust_sched_loop::return_c_stack(stk_seg *stack) {
 // NB: Runs on the Rust stack. Might return NULL!
 inline stk_seg *
 rust_sched_loop::borrow_big_stack() {
-    assert(cached_big_stack);
     stk_seg *your_stack;
     if (extra_big_stack) {
         your_stack = extra_big_stack;
         extra_big_stack = NULL;
     } else {
+        // NB: This may be null if we're asking for a *second*
+        // big stack, in which case the caller will fall back to a slow path
         your_stack = cached_big_stack;
         cached_big_stack = NULL;
     }

diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 81ae991623f8c7f06d8a5b3a9f4961d65ab086aa..4d6d2567cd4525bb532d4f130d0d268466eedd9e 100644 (file)
--- a/src/rt/rust_task.cpp
+++ b/src/rt/rust_task.cpp
@@ -54,8 +54,7 @@ rust_task::rust_task(rust_sched_loop *sched_loop, rust_task_state state,
     disallow_yield(0),
     c_stack(NULL),
     next_c_sp(0),
-    next_rust_sp(0),
-    big_stack(NULL)
+    next_rust_sp(0)
 {
     LOGPTR(sched_loop, "new task", (uintptr_t)this);
     DLOG(sched_loop, task, "sizeof(task) = %d (0x%x)",
@@ -566,14 +565,8 @@ rust_task::cleanup_after_turn() {
 
     while (stk->next) {
         stk_seg *new_next = stk->next->next;
-
-        if (stk->next->is_big) {
-            assert (big_stack == stk->next);
-            sched_loop->return_big_stack(big_stack);
-            big_stack = NULL;
-        } else {
-            free_stack(stk->next);
-        }
+        assert (!stk->next->is_big);
+        free_stack(stk->next);
 
         stk->next = new_next;
     }
@@ -584,38 +577,20 @@ rust_task::cleanup_after_turn() {
 bool
 rust_task::new_big_stack() {
     assert(stk);
-    // If we have a cached big stack segment, use it.
-    if (big_stack) {
-        // Check to see if we're already on the big stack.
-        stk_seg *ss = stk;
-        while (ss != NULL) {
-            if (ss == big_stack)
-                return false;
-            ss = ss->prev;
-        }
 
-        // Unlink the big stack.
-        if (big_stack->next)
-            big_stack->next->prev = big_stack->prev;
-        if (big_stack->prev)
-            big_stack->prev->next = big_stack->next;
-    } else {
-        stk_seg *borrowed_big_stack = sched_loop->borrow_big_stack();
-        if (!borrowed_big_stack) {
-            abort();
-        } else {
-            big_stack = borrowed_big_stack;
-        }
+    stk_seg *borrowed_big_stack = sched_loop->borrow_big_stack();
+    if (!borrowed_big_stack) {
+        return false;
     }
 
-    big_stack->task = this;
-    big_stack->next = stk->next;
-    if (big_stack->next)
-        big_stack->next->prev = big_stack;
-    big_stack->prev = stk;
-    stk->next = big_stack;
+    borrowed_big_stack->task = this;
+    borrowed_big_stack->next = stk->next;
+    if (borrowed_big_stack->next)
+        borrowed_big_stack->next->prev = borrowed_big_stack;
+    borrowed_big_stack->prev = stk;
+    stk->next = borrowed_big_stack;
 
-    stk = big_stack;
+    stk = borrowed_big_stack;
 
     return true;
 }
@@ -640,10 +615,9 @@ void
 rust_task::reset_stack_limit() {
     uintptr_t sp = get_sp();
     while (!sp_in_stk_seg(sp, stk)) {
-        stk = stk->prev;
+        prev_stack();
         assert(stk != NULL && "Failed to find the current stack");
     }
-    record_stack_limit();
 }
 
 void
@@ -667,8 +641,6 @@ rust_task::delete_all_stacks() {
 
         stk = prev;
     }
-
-    big_stack = NULL;
 }
 
 /*

diff --git a/src/rt/rust_task.h b/src/rt/rust_task.h
index 982102cdde0cd3c2773bd02ccfcd3dfa2db4364a..1735d35b0652366a33d7ae91177cadadd7de913a 100644 (file)
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -278,9 +278,6 @@ private:
     uintptr_t next_c_sp;
     uintptr_t next_rust_sp;
 
-    // The big stack.
-    stk_seg *big_stack;
-
     // Called when the atomic refcount reaches zero
     void delete_this();
 
@@ -607,7 +604,21 @@ rust_task::prev_stack() {
     // require switching to the C stack and be costly. Instead we'll just move
     // up the link list and clean up later, either in new_stack or after our
     // turn ends on the scheduler.
-    stk = stk->prev;
+    if (stk->is_big) {
+        stk_seg *ss = stk;
+        stk = stk->prev;
+
+        // Unlink the big stack.
+        if (ss->next)
+            ss->next->prev = ss->prev;
+        if (ss->prev)
+            ss->prev->next = ss->next;
+
+        sched_loop->return_big_stack(ss);
+    } else {
+        stk = stk->prev;
+    }
+
     record_stack_limit();
 }
 

diff --git a/src/test/bench/shootout-threadring.rs b/src/test/bench/shootout-threadring.rs
index 213c5140ccf0c3059724a5ccd49b7cabce22bc18..a67bbb05dfb51886ee7d4cae3789488b61f755f4 100644 (file)
--- a/src/test/bench/shootout-threadring.rs
+++ b/src/test/bench/shootout-threadring.rs
@@ -10,18 +10,18 @@
 
 // Based on threadring.erlang by Jira Isa
 
-// xfail-test FIXME #5985 OOM's on the mac bot
+use std::os;
 
 fn start(n_tasks: int, token: int) {
-    let mut (p, ch1) = comm::stream();
+    let mut (p, ch1) = stream();
     ch1.send(token);
     //  XXX could not get this to work with a range closure
     let mut i = 2;
     while i <= n_tasks {
-        let (next_p, ch) = comm::stream();
+        let (next_p, ch) = stream();
         let imm_i = i;
         let imm_p = p;
-        do task::spawn {
+        do spawn {
             roundtrip(imm_i, n_tasks, &imm_p, &ch);
         };
         p = next_p;
@@ -29,16 +29,16 @@ fn start(n_tasks: int, token: int) {
     }
     let imm_p = p;
     let imm_ch = ch1;
-    do task::spawn {
+    do spawn {
         roundtrip(1, n_tasks, &imm_p, &imm_ch);
     }
 }
 
-fn roundtrip(id: int, n_tasks: int, p: &comm::Port<int>, ch: &comm::Chan<int>) {
+fn roundtrip(id: int, n_tasks: int, p: &Port<int>, ch: &Chan<int>) {
     while (true) {
         match p.recv() {
           1 => {
-            io::println(fmt!("%d\n", id));
+            println(fmt!("%d\n", id));
             return;
           }
           token => {
@@ -60,13 +60,13 @@ fn main() {
         os::args()
     };
     let token = if args.len() > 1u {
-        int::from_str(args[1]).get()
+        FromStr::from_str(args[1]).get()
     }
     else {
         1000
     };
     let n_tasks = if args.len() > 2u {
-        int::from_str(args[2]).get()
+        FromStr::from_str(args[2]).get()
     }
     else {
         503