Skip to content

io_uring: give Hijack a synchronous detach-queue round trip so it works under AsyncHandlers #558

Description

@FumingPower3925

Follow-up to #539, which refused Hijack on the async dispatch path rather than making it work.
That was the safe call for v1.6.0; this is the complete fix.

Why refusing is not the end state

hijackConn is worker-owned work — w.conns, w.connCount, w.liveConns, the dirty list, and an
ASYNC_CANCEL SQE under single-issuer. In async mode the handler runs on the per-connection
dispatch goroutine, so it cannot do that work itself. Every other goroutine-to-worker hand-off in
the engine goes through detachQueue for exactly this reason.

Hijack is the one that cannot use it as-is, because Hijack() is synchronous:
internal/conn/response.go calls hijackFn and returns its result straight to the handler. So it
needs a hand-off that blocks the caller until the worker has done the work and produced a
net.Conn.

Sketch

  • A request struct on the connState — done chan struct{} plus the net.Conn and error the
    worker fills in.
  • HijackFn branches on w.async. Sync mode must keep calling hijackConn directly: the
    handler already runs on the worker thread there, so a round trip would deadlock the worker against
    itself.
  • Async mode publishes the request, enqueues the conn on detachQueue, signals the eventfd, and
    waits.
  • drainDetachQueue performs hijackConn on the worker thread, fills the result, closes done.

The part that needs care

Shutdown. If the worker stops before servicing the request, the handler goroutine waits forever.
w.runCtx is cancellable (the engine builds it with context.WithCancel), so the wait should be

select {
case <-req.done:
case <-w.runCtx.Done():
    return nil, errors.New("celeris: engine shutting down")
}

and shutdown() should additionally drain any queued hijack requests and fail them, so the error is
delivered deterministically rather than depending on cancellation ordering.

Test

The combination has no coverage at all today: an io_uring engine with AsyncHandlers enabled and a
handler that calls ctx.Hijack(). Worth adding regardless of which fix lands, including the
shutdown race — a hijack request in flight while the engine stops.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions