From 2386988123ce9a9ca797e48732bcf69cde412fdd Mon Sep 17 00:00:00 2001
From: Brian Anderson <banderson@mozilla.com>
Date: Fri, 17 May 2013 17:46:01 -0700
Subject: [PATCH] rt: Release big stacks immediately after use to avoid holding
 on to them 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
---
 src/rt/rust_task.cpp                  | 57 +++++++--------------------
 src/rt/rust_task.h                    | 19 +++++++--
 src/test/bench/shootout-threadring.rs |  2 -
 3 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp
index 266c0652c6e59..3b6ce56ddab3e 100644
--- 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)",
@@ -564,14 +563,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;
     }
@@ -581,39 +574,20 @@ rust_task::cleanup_after_turn() {
 // stack and false otherwise.
 bool
 rust_task::new_big_stack() {
-    // 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;
+    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;
     if (stk)
-        stk->next = big_stack;
+        stk->next = borrowed_big_stack;
 
-    stk = big_stack;
+    stk = borrowed_big_stack;
 
     return true;
 }
@@ -638,10 +612,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
@@ -665,8 +638,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 672af608db863..ac3d3d212dd39 100644
--- a/src/rt/rust_task.h
+++ b/src/rt/rust_task.h
@@ -275,9 +275,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();
 
@@ -604,7 +601,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 213c5140ccf0c..9eb8a29cec030 100644
--- a/src/test/bench/shootout-threadring.rs
+++ b/src/test/bench/shootout-threadring.rs
@@ -10,8 +10,6 @@
 
 // Based on threadring.erlang by Jira Isa
 
-// xfail-test FIXME #5985 OOM's on the mac bot
-
 fn start(n_tasks: int, token: int) {
     let mut (p, ch1) = comm::stream();
     ch1.send(token);