Skip to content

Commit

Permalink
Avoid stack overflow error with Future.after
Browse files Browse the repository at this point in the history
With a long list of resolved futures, the previous implementation
caused a stack-overflow exception, as each call to await_next happened
within the previous call (synchronously).

This changes the behavior to loop over futures instead of using
recursion while on_complete dispatches synchronously in the same thread.
To avoid code duplication we always dispatch in the terminating cases,
even if it introduces an extra stack frame. The looping condition
protects against cases where a future would be resolved at a later
point by the calling thread (which is what the tests do), and the last
part of the condition protects against dispatching in other threads.

While the algorithm looks like a loop that adds multiple concurrent
on_complete listeners, the intention is in fact still to only ever have
one concurrent listener, making sure that the combined nil future is
only ever accessed from one thread at a time (without using locks).
  • Loading branch information
grddev committed Jul 14, 2015
1 parent 560ad4d commit 9c12c9c
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
14 changes: 13 additions & 1 deletion lib/ione/future.rb
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,19 @@ def await_next(v, e)
@futures = nil
resolve
else
@futures.pop.on_complete(&method(:await_next))
outer = Thread.current
looping = more = true
while more
more = false
@futures.pop.on_complete do |v, e|
if e || @futures.empty? || !looping || !Thread.current.equal?(outer)
await_next(v, e)
else
more = true
end
end
end
looping = false
end
end
end
Expand Down
6 changes: 6 additions & 0 deletions spec/ione/future_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,12 @@ def delayed(*context, &listener)
future = Future.after(ff1, ff2, ff3)
future.value.should be_nil
end

it 'handles a really long list of futures' do
futures = Array.new(10000, Future.resolved)
future = Future.after(futures)
future.value.should be_nil
end
end
end

Expand Down

0 comments on commit 9c12c9c

Please sign in to comment.