fix: keep a hijacked request's receive buffer out of the pool on epoll and io_uring, and copy the request values at Hijack (celeris#733)

.github/workflows/ci.yml

@@ -599,6 +599,34 @@ jobs:
             echo "expected all four tests and all 20 arms (8 of them io_uring) to PASS with no SKIP line"
             exit 1
           fi
+      # celeris#733: the request strings a hijacking handler keeps must not
+      # read another connection's bytes. The root race step above runs
+      # without -v, where the io_uring arm of TestHijackKeepsRequestViews
+      # drops out, and the multishot test skips, without a word when the
+      # probe finds no ring or no multishot receive. So both run here by
+      # name with CELERIS_REQUIRE_IOURING_WORKERS=1, and an exact tally: the
+      # two top-level tests and the four arms (std, epoll, epoll-async,
+      # io_uring) must PASS, with no SKIP line, and go test must exit 0.
+      - name: celeris#733 hijacked request strings (io_uring arms required)
+        if: ${{ !cancelled() }}
+        shell: bash
+        env:
+          CELERIS_REQUIRE_IOURING_WORKERS: "1"
+        run: |
+          set -o pipefail
+          echo "memlock (KiB): $(ulimit -l)"
+          failed=0
+          tests='TestHijackKeepsRequestViews|TestHijackCopiesRequestValuesUnderMultishotRecv'
+          go test -race -count=1 -timeout=300s -v -run "^(${tests})\$" . 2>&1 | tee /tmp/c733.log || failed=1
+          top=$(grep -cE "^--- PASS: (${tests}) \(" /tmp/c733.log || true)
+          arms=$(grep -cE "^    --- PASS: TestHijackKeepsRequestViews/" /tmp/c733.log || true)
+          uring=$(grep -cE "^    --- PASS: TestHijackKeepsRequestViews/io_uring" /tmp/c733.log || true)
+          skipped=$(grep -cE '^[[:space:]]*--- SKIP' /tmp/c733.log || true)
+          echo "celeris#733 tests: top-level PASS $top (want 2), arm PASS $arms (want 4), io_uring arm PASS $uring (want 1), SKIP lines $skipped, go test failed $failed (want 0)"
+          if [ "$top" -ne 2 ] || [ "$arms" -ne 4 ] || [ "$uring" -ne 1 ] || [ "$skipped" -ne 0 ] || [ "$failed" -ne 0 ]; then
+            echo "expected both tests and all four arms (one io_uring) to PASS with no SKIP line"
+            exit 1
+          fi
 
   adaptive:
     name: Adaptive engine (./adaptive/...)

context_response.go

@@ -1256,6 +1256,17 @@ func (c *Context) BytesWritten() int {
 // owns the connection and is responsible for closing it. Supported on all
 // engines for HTTP/1.1 connections. HTTP/2 connections cannot be hijacked
 // (multiplexed streams share a single TCP connection).
+//
+// The request's strings stay valid after Hijack, for the handler and for
+// any goroutine it hands them to (celeris#733). On epoll and io_uring they
+// are views of the connection's receive buffer, which the engine does not
+// give to another connection once the connection is hijacked. Hijack also
+// copies the request values the Context holds, as [Context.Detach] does, so
+// the path, params, headers, query, cookies and body read from the Context
+// after Hijack are copies. In io_uring's opt-in multishot receive mode
+// (CELERIS_IOURING_MULTISHOT_RECV=1) the request is received into a buffer
+// the engine hands back to the kernel when the handler returns: there, keep
+// only strings read after Hijack, or clone the ones read before it.
 func (c *Context) Hijack() (net.Conn, error) {
 	if c.written {
 		return nil, errors.New("celeris: cannot hijack after response written")
@@ -1264,6 +1275,9 @@ func (c *Context) Hijack() (net.Conn, error) {
 	if !ok {
 		return nil, ErrHijackNotSupported
 	}
+	// Before the engine lets go of the connection, and so of its receive
+	// buffer (celeris#733).
+	c.cloneRequestValues()
 	conn, err := h.Hijack(c.stream)
 	if err != nil {
 		return nil, err
@@ -1420,25 +1434,7 @@ func (c *Context) Detach() (done func()) {
 	if c.detached {
 		return func() {} // already detached — return no-op done
 	}
-	// Materialize any unsafe string headers (zero-copy H1 headers backed by
-	// the connection's read buffer) before the handler returns and the buffer
-	// is reused for the next recv. Pseudo-header keys are string literals
-	// (safe), but their values (:authority, :path for non-"/" paths, :method
-	// for non-standard methods) may be UnsafeString backed by the buffer.
-	c.stream.MaterializeHeaders()
-	for i, h := range c.stream.Headers {
-		if len(h[0]) > 0 && h[0][0] == ':' {
-			c.stream.Headers[i][1] = strings.Clone(h[1])
-			continue
-		}
-		c.stream.Headers[i][0] = strings.Clone(h[0])
-		c.stream.Headers[i][1] = strings.Clone(h[1])
-	}
-	// Also materialize extracted fields that may reference the buffer.
-	c.method = strings.Clone(c.method)
-	c.path = strings.Clone(c.path)
-	c.rawQuery = strings.Clone(c.rawQuery)
-	c.materializeRequestViews()
+	c.cloneRequestValues()
 
 	c.extended = true
 	c.detached = true
@@ -1462,9 +1458,36 @@ func (c *Context) Detach() (done func()) {
 	}
 }
 
-// materializeRequestViews clones, for [Context.Detach], the request-derived
-// strings the Context keeps outside the header slice and the method, path and
-// raw query: the route params, the parsed query and cookie caches, the H1
+// cloneRequestValues copies every request value the Context holds that can be
+// a view of the engine's receive buffer, for [Context.Detach] and
+// [Context.Hijack]: after either, the handler, or a goroutine it starts, can
+// keep using those values after the engine has received into, or given
+// away, the buffer they view.
+func (c *Context) cloneRequestValues() {
+	// Materialize any unsafe string headers (zero-copy H1 headers backed by
+	// the connection's read buffer) before the handler returns and the buffer
+	// is reused for the next recv. Pseudo-header keys are string literals
+	// (safe), but their values (:authority, :path for non-"/" paths, :method
+	// for non-standard methods) may be UnsafeString backed by the buffer.
+	c.stream.MaterializeHeaders()
+	for i, h := range c.stream.Headers {
+		if len(h[0]) > 0 && h[0][0] == ':' {
+			c.stream.Headers[i][1] = strings.Clone(h[1])
+			continue
+		}
+		c.stream.Headers[i][0] = strings.Clone(h[0])
+		c.stream.Headers[i][1] = strings.Clone(h[1])
+	}
+	// Also materialize extracted fields that may reference the buffer.
+	c.method = strings.Clone(c.method)
+	c.path = strings.Clone(c.path)
+	c.rawQuery = strings.Clone(c.rawQuery)
+	c.materializeRequestViews()
+}
+
+// materializeRequestViews clones, for [Context.Detach] and [Context.Hijack],
+// the request-derived strings the Context keeps outside the header slice and
+// the method, path and raw query: the route params, the parsed query and cookie caches, the H1
 // Host, the strings middleware store on the Context from request headers
 // (request ID, client-IP/host/scheme overrides, SetString values), the
 // request body, and the response headers set so far.

engine/epoll/loop.go

@@ -1779,6 +1779,15 @@ func (l *Loop) hijackConn(fd int) (net.Conn, error) {
 		// drainDetachQueue, which also runs there.
 		l.removeDirty(cs)
 		l.dropAsk(cs) // celeris#657 P8: never pool a connState an ask still names
+		// The request's strings are views of cs.buf (celeris#733). The
+		// handler is still running, and a hijacking handler typically keeps
+		// the path, params and headers for the goroutine that will serve
+		// the connection, so the buffer must not go back to the pool with
+		// cs: the next connection to take cs from the pool would receive its
+		// request into it, on any worker, even before this handler returns.
+		// Dropping it costs one buffer allocation per hijack, when that
+		// connState is next acquired.
+		cs.buf = nil
 		releaseConnState(cs)
 	}
 	return c, err

engine/iouring/worker.go

@@ -2245,9 +2245,20 @@ func (w *Worker) hijackConn(fd int) (net.Conn, error) {
 	// STEAL the first bytes the hijacker tries to read. Cancel it by its
 	// generation-tagged user_data and defer the pool release until the
 	// terminal CQE arrives, exactly like finishClose.
+	//
+	// Then drop cs instead of recycling it (celeris#733): the request's
+	// strings are views of cs.buf (or of cs.detectAccum, when the request
+	// began in an earlier recv), and a hijacking handler typically keeps
+	// the path, params and headers for the goroutine that serves the
+	// connection. Recycled, cs would hand those buffers to the next
+	// connection this worker accepts, which would receive its request into
+	// them. The detached release holds cs until the kernel is done with
+	// cs.buf and then leaves it to the garbage collector, which frees the
+	// buffers once the last view of them is gone. The cost is one connState
+	// allocation per hijack.
 	w.cancelConnOps(fd, cs)
 	w.noteClosedInflight(cs)
-	w.queuePendingRelease(cs)
+	w.queuePendingReleaseDetached(cs)
 	f := os.NewFile(uintptr(fd), "tcp")
 	c, err := net.FileConn(f)
 	_ = f.Close()