Skip to content

Commit

Permalink
Begin the shutdown process before releasing the last waiter.
Browse files Browse the repository at this point in the history
Once we push the final result on the resume queue the waiting
thread may immediately resume, and see a Fiber that appears to
be alive but which will actually soon be dead. By moving one of
the death conditions before the final push, we will at least
ensure the Fiber appears dead.

There is an opposite side effect here, in that now a Fiber may
appear to be dead a brief moment before it returns its last
value, but this can only be observed from a third thread, which
could not atomically know that both the final value had returned
and the Fiber had declared itself dead.

Fixes #4838.
  • Loading branch information
headius committed Nov 21, 2017
1 parent 8d5b2ca commit ce61601
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions core/src/main/java/org/jruby/ext/fiber/ThreadFiber.java
Expand Up @@ -275,10 +275,19 @@ public void run() {
result = block.yieldArray(context, init, null);
}

// Clear ThreadFiber's thread since we're on the way out and need to appear non-alive?
// Waiting thread can proceed immediately after push below but before we are truly dead.
// See https://github.com/jruby/jruby/issues/4838
ThreadFiber tf = data.fiber.get();
if (tf != null) tf.thread = null;

data.prev.data.queue.push(context, new IRubyObject[]{result});
} finally {
// Ensure we do everything for shutdown now
data.queue.shutdown();
runtime.getThreadService().disposeCurrentThread();
ThreadFiber tf = data.fiber.get();
if (tf != null) tf.thread = null;
}
} catch (JumpException.FlowControlException fce) {
if (data.prev != null) {
Expand All @@ -304,10 +313,6 @@ public void run() {
if (data.prev != null) {
data.prev.thread.raise(JavaUtil.convertJavaToUsableRubyObject(runtime, t));
}
} finally {
// clear reference to the fiber's thread
ThreadFiber tf = data.fiber.get();
if (tf != null) tf.thread = null;
}
}
});
Expand Down

0 comments on commit ce61601

Please sign in to comment.