auto merge of #7111 : brson/rust/stack, r=brson

... through yields

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

(reopening after incoming fallout. *do not r+*. broken)

Author: bors

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

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;
 }
 
 /*

src/rt/rust_task.h

@@ -278,9 +278,6 @@ rust_task : public kernel_owned<rust_task>
     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();
 }
 

src/test/bench/shootout-threadring.rs

@@ -10,35 +10,35 @@
 
 // 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;
         i += 1;
     }
     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