Skip to content

[Bug] Fiber activity executor: cancelling an activity permanently strands the poller under a fiber scheduler #570

Description

@davidalejandroaguilar

What are you really trying to do?

Run long-lived workflows whose activities spend most of their time awaiting slow external I/O (model/API calls, DB writes), on the fiber activity executor under an Async reactor so one worker serves many workflows concurrently. In this workload, newer input routinely supersedes work still in flight, so cancelling a running activity is an everyday event for us, not an edge case.

Describe the bug

Temporalio::Worker::ActivityExecutor::Fiber#set_activity_context cancels an
activity with Fiber#raise. Under a fiber scheduler that uses Fiber#transfer
to switch fibers (Async, the scheduler the fiber executor is designed to be used
with), Fiber#raise moves control INTO the target and the caller never gets it
back.

The caller is the worker's poller loop. So the first cancelled activity strands
it permanently: the reactor stays up, the process looks healthy, every thread is
idle, and the worker never processes another task on any workflow until it is
restarted. Nothing is raised, nothing is logged.

Where:

lib/temporalio/worker/activity_executor/fiber.rb:

def set_activity_context(defn, context)
  ::Fiber[:temporal_activity_context] = context
  return unless defn.cancel_raise

  fiber = ::Fiber.current
  context&.cancellation&.add_cancel_callback do
    fiber.raise(Error::CanceledError.new('Activity canceled'))
  end
end

Why it happens:

Async drives task fibers with Fiber#transfer, and Fiber#raise on a
transferred fiber uses transfer semantics. Control moves into the target; when
the target next blocks it transfers to the SELECTOR, not back to the raiser. The
raiser is on no ready list and in no wait queue, so nothing ever resumes it.

In the worker this is reached from handle_cancel_task on the poller fiber, so
one cancelled activity ends the worker's ability to process anything. For an
workload where a workflow routinely cancels an in-flight activity
(a user sending a second message while a step is running), this is an ordinary
path rather than an edge case.

Expected:

Cancelling a fiber-executor activity raises CanceledError in the activity and
leaves the canceller running.

Minimal Reproduction

No Temporal server needed. This is the executor's cancellation shape reduced to
two fibers under Async.

require "async"

Async do |task|
  target = Fiber.schedule do
    sleep(5)
  rescue => e
    $stderr.puts "TARGET: rescued #{e.class}"
    sleep(0.05)                     # the activity reports its completion here
    $stderr.puts "TARGET: finished"
  end

  task.sleep(0.2)
  $stderr.puts "RAISER: raising"
  target.raise(RuntimeError.new("cancel"))
  $stderr.puts "RAISER: control came back"   # NEVER PRINTS
end

$stderr.puts "REACTOR: exited"               # NEVER PRINTS

Observed (timeout 60 ruby repro.rb; exits 124):

RAISER: raising
TARGET: rescued RuntimeError
TARGET: finished

RAISER: control came back and REACTOR: exited never print. The raiser is
suspended with nothing holding a reference to it, and the reactor never winds
down.

The second sleep in the target is load-bearing and is not artificial: a real
activity, after unwinding, reports its completion through the bridge, which
blocks. That is what transfers control to the selector instead of back to the
raiser. A target that unwinds and returns immediately can mask the bug.

Environment/Versions

Reproduced independently on two stacks:

Ruby temporalio async
Reporter 3.4.7 1.7.0 2.45.1
Second reproduction 3.4.7 1.6.0 2.39.0

The stock executor is unchanged in 1.7.0, the latest release at the time of
writing, so this is not fixed upstream.

Additional context

Fix that works for us (shipped as an application-level prepend):

Route the raise through the scheduler when it offers one. Async's own
cancellation (Async::Task#cancel) does exactly this rather than calling
Fiber#raise directly:

context&.cancellation&.add_cancel_callback do
  error = Error::CanceledError.new('Activity canceled')
  scheduler = ::Fiber.scheduler
  if scheduler.respond_to?(:raise)
    scheduler.raise(fiber, error)
  else
    fiber.raise(error)
  end
end

Async::Scheduler#raise delegates to the selector, and the selector's
implementation is the reason this is correct rather than merely different
(io-event, Selector#raise):

def raise(fiber, *arguments, **options)
  optional = Optional.new(Fiber.current)
  @ready.push(optional)                 # enqueue the CALLER ...
  fiber.raise(*arguments, **options)    # ... and only then transfer
ensure
  optional.nullify
end

The caller is put on the event loop's ready list before it gives up control, so
when the target eventually blocks and transfers to the selector, the selector
has the caller to resume. Nothing about the bare raise supplies that.

Running the reproduction above through Fiber.scheduler.raise(target, ...)
prints all four lines and exits 0.

Note that Fiber::Scheduler#raise is not a Ruby scheduler hook, so the
respond_to? guard matters: a scheduler that does not offer one keeps today's
behaviour rather than crashing.

Workaround:

Prepending the corrected set_activity_context onto
Temporalio::Worker::ActivityExecutor::Fiber works and covers
Temporalio::Testing::ActivityEnvironment as well, since it resolves the same
executor instance. Passing a corrected subclass through Worker.new's
activity_executors: option also works but reaches only workers you construct;
ActivityEnvironment takes its own separate activity_executors: keyword and
does not see the worker's.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions