Skip to content

Commit 2386988

Browse files
committed
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
1 parent 2d28d64 commit 2386988

File tree

3 files changed

+29
-49
lines changed

3 files changed

+29
-49
lines changed

src/rt/rust_task.cpp

+14-43
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ rust_task::rust_task(rust_sched_loop *sched_loop, rust_task_state state,
5454
disallow_yield(0),
5555
c_stack(NULL),
5656
next_c_sp(0),
57-
next_rust_sp(0),
58-
big_stack(NULL)
57+
next_rust_sp(0)
5958
{
6059
LOGPTR(sched_loop, "new task", (uintptr_t)this);
6160
DLOG(sched_loop, task, "sizeof(task) = %d (0x%x)",
@@ -564,14 +563,8 @@ rust_task::cleanup_after_turn() {
564563

565564
while (stk->next) {
566565
stk_seg *new_next = stk->next->next;
567-
568-
if (stk->next->is_big) {
569-
assert (big_stack == stk->next);
570-
sched_loop->return_big_stack(big_stack);
571-
big_stack = NULL;
572-
} else {
573-
free_stack(stk->next);
574-
}
566+
assert (!stk->next->is_big);
567+
free_stack(stk->next);
575568

576569
stk->next = new_next;
577570
}
@@ -581,39 +574,20 @@ rust_task::cleanup_after_turn() {
581574
// stack and false otherwise.
582575
bool
583576
rust_task::new_big_stack() {
584-
// If we have a cached big stack segment, use it.
585-
if (big_stack) {
586-
// Check to see if we're already on the big stack.
587-
stk_seg *ss = stk;
588-
while (ss != NULL) {
589-
if (ss == big_stack)
590-
return false;
591-
ss = ss->prev;
592-
}
593-
594-
// Unlink the big stack.
595-
if (big_stack->next)
596-
big_stack->next->prev = big_stack->prev;
597-
if (big_stack->prev)
598-
big_stack->prev->next = big_stack->next;
599-
} else {
600-
stk_seg *borrowed_big_stack = sched_loop->borrow_big_stack();
601-
if (!borrowed_big_stack) {
602-
abort();
603-
} else {
604-
big_stack = borrowed_big_stack;
605-
}
577+
stk_seg *borrowed_big_stack = sched_loop->borrow_big_stack();
578+
if (!borrowed_big_stack) {
579+
return false;
606580
}
607581

608-
big_stack->task = this;
609-
big_stack->next = stk->next;
610-
if (big_stack->next)
611-
big_stack->next->prev = big_stack;
612-
big_stack->prev = stk;
582+
borrowed_big_stack->task = this;
583+
borrowed_big_stack->next = stk->next;
584+
if (borrowed_big_stack->next)
585+
borrowed_big_stack->next->prev = borrowed_big_stack;
586+
borrowed_big_stack->prev = stk;
613587
if (stk)
614-
stk->next = big_stack;
588+
stk->next = borrowed_big_stack;
615589

616-
stk = big_stack;
590+
stk = borrowed_big_stack;
617591

618592
return true;
619593
}
@@ -638,10 +612,9 @@ void
638612
rust_task::reset_stack_limit() {
639613
uintptr_t sp = get_sp();
640614
while (!sp_in_stk_seg(sp, stk)) {
641-
stk = stk->prev;
615+
prev_stack();
642616
assert(stk != NULL && "Failed to find the current stack");
643617
}
644-
record_stack_limit();
645618
}
646619

647620
void
@@ -665,8 +638,6 @@ rust_task::delete_all_stacks() {
665638

666639
stk = prev;
667640
}
668-
669-
big_stack = NULL;
670641
}
671642

672643
/*

src/rt/rust_task.h

+15-4
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,6 @@ rust_task : public kernel_owned<rust_task>
275275
uintptr_t next_c_sp;
276276
uintptr_t next_rust_sp;
277277

278-
// The big stack.
279-
stk_seg *big_stack;
280-
281278
// Called when the atomic refcount reaches zero
282279
void delete_this();
283280

@@ -604,7 +601,21 @@ rust_task::prev_stack() {
604601
// require switching to the C stack and be costly. Instead we'll just move
605602
// up the link list and clean up later, either in new_stack or after our
606603
// turn ends on the scheduler.
607-
stk = stk->prev;
604+
if (stk->is_big) {
605+
stk_seg *ss = stk;
606+
stk = stk->prev;
607+
608+
// Unlink the big stack.
609+
if (ss->next)
610+
ss->next->prev = ss->prev;
611+
if (ss->prev)
612+
ss->prev->next = ss->next;
613+
614+
sched_loop->return_big_stack(ss);
615+
} else {
616+
stk = stk->prev;
617+
}
618+
608619
record_stack_limit();
609620
}
610621

src/test/bench/shootout-threadring.rs

-2
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010

1111
// Based on threadring.erlang by Jira Isa
1212

13-
// xfail-test FIXME #5985 OOM's on the mac bot
14-
1513
fn start(n_tasks: int, token: int) {
1614
let mut (p, ch1) = comm::stream();
1715
ch1.send(token);

0 commit comments

Comments
 (0)