You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-ups from PR #723 (Fixes #714, #717, #718), deferred under the maintainer's two-round review cap (2026-09-27). Round 2 of the independent review approved #723 at c98f068 with minors only; they are recorded here. Each was checked against c98f068.
otel carrier: the traceparent clone buys nothing, and the doc names too narrow a reason.headerCarrier.Get (middleware/otel/carrier.go:37) clones every propagation header that is present, including traceparent. The W3C TraceContext propagator hex-decodes traceparent into fixed-size TraceID and SpanID arrays and keeps no substring, so that copy is a per-request allocation with no benefit: the PR's Cost table shows the traced request going from 16 allocs / 2313 B to 17 allocs / 2377 B, and traceparent + tracestate + baggage from 27 to 30 allocs. The clone is justified for tracestate and baggage, and more broadly than the doc comment (carrier.go:22-31) says: the SDK samplers copy the parent's TraceState into the server span and the batch processor exports it after the request ends, so a view in tracestate outlives every sampled request, not only detached streams. Either skip the clone for traceparent, or keep it and say in the doc that it is deliberate; fix the doc either way. Keys() also clones each name separately (carrier.go:57); one buffer, as cloneHeaderPairs does, would make it one allocation per call.
Session middleware stores cookie views with c.Set. The audit's sweep covered first-party SetContext/WithValue, not first-party c.Set values, which the new Detach doc says the storer must clone. middleware/session/session.go:763/:765 set sess.id and sess.presentedID to the cookie view, and :784 stores sess with c.Set(ContextKey, sess). A detached SSE OnDisconnect(c, client), or any detached goroutine calling session.FromContext(c).ID() or Save, reads the reused buffer on epoll and io_uring. This is the session class of middleware/session: with WriteBehind on epoll and io_uring, a loaded session is saved under the NEXT request's session ID (the queued id is a view of the receive buffer): cross-session write #731; check whether its fix covers these two fields, and add the row to the audit next to the jwt Token.Raw note.
The PR body's ENOMEM sentence is broader than its evidence.fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723's CI section says "No ENOMEM retry fired in this run, the two red runs, or any local run." The counted runs and the three CI Unit jobs are clean (0 server start retried lines; every io_uring listener reported tier=high), but the server-start ENOMEM retry did fire, 3 to 15 tries, in about 12 superseded local mutant runs. The probe retry (c714ProbeIOUring, detach_capture_alias_linux_test.go:323, and the same helper in the ws, sse and otel tests) never logs, so no log can show whether it fired, locally or in CI. Make the probe retry t.Logf when it fires, so CI can show it; the sentence should have been limited to the counted runs.
Not carried over: round 2 also asked whether #723's public body should keep the sentence about the multishot receive mode reading another connection's bytes. The maintainer chose public handling of that class (#731, #732, #733), so it stays.
Follow-ups from PR #723 (Fixes #714, #717, #718), deferred under the maintainer's two-round review cap (2026-09-27). Round 2 of the independent review approved #723 at c98f068 with minors only; they are recorded here. Each was checked against c98f068.
otel carrier: the
traceparentclone buys nothing, and the doc names too narrow a reason.headerCarrier.Get(middleware/otel/carrier.go:37) clones every propagation header that is present, includingtraceparent. The W3C TraceContext propagator hex-decodestraceparentinto fixed-size TraceID and SpanID arrays and keeps no substring, so that copy is a per-request allocation with no benefit: the PR's Cost table shows the traced request going from 16 allocs / 2313 B to 17 allocs / 2377 B, andtraceparent + tracestate + baggagefrom 27 to 30 allocs. The clone is justified fortracestateandbaggage, and more broadly than the doc comment (carrier.go:22-31) says: the SDK samplers copy the parent's TraceState into the server span and the batch processor exports it after the request ends, so a view intracestateoutlives every sampled request, not only detached streams. Either skip the clone fortraceparent, or keep it and say in the doc that it is deliberate; fix the doc either way.Keys()also clones each name separately (carrier.go:57); one buffer, ascloneHeaderPairsdoes, would make it one allocation per call.SSE
Config.OnConnecthands out views that a detached stream keeps.OnConnectis documented as the place to extract request metadata (middleware/sse/config.go:102-105) and runs beforeDetach(middleware/sse/sse.go:373vs:426). A string a user keeps from it (a hub subscription keyed byc.Paramorc.Query, a user ID stored for the stream) is a view the stream keeps for its whole life: middleware/websocket: on epoll and io_uring, Conn.Query returns garbage once a frame has arrived, because the captured query strings alias the engine's read buffer (200/200 per engine) #714's mechanism, reached through an in-tree API. The newDetachdoc says strings read before the call cannot be copied, but an SSE user cannot move the read afterDetach, because the middleware callsDetachitself. Fix by warning inOnConnect's doc that kept values must be cloned, or by materializing the request views beforeonConnectruns. The fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723 audit did not list this callback.Session middleware stores cookie views with
c.Set. The audit's sweep covered first-partySetContext/WithValue, not first-partyc.Setvalues, which the newDetachdoc says the storer must clone.middleware/session/session.go:763/:765setsess.idandsess.presentedIDto the cookie view, and:784storessesswithc.Set(ContextKey, sess). A detached SSEOnDisconnect(c, client), or any detached goroutine callingsession.FromContext(c).ID()orSave, reads the reused buffer on epoll and io_uring. This is the session class of middleware/session: with WriteBehind on epoll and io_uring, a loaded session is saved under the NEXT request's session ID (the queued id is a view of the receive buffer): cross-session write #731; check whether its fix covers these two fields, and add the row to the audit next to the jwtToken.Rawnote.middleware/websocket: on epoll and io_uring, Conn.Query returns garbage once a frame has arrived, because the captured query strings alias the engine's read buffer (200/200 per engine) #714, middleware/sse: on epoll and io_uring, Client.LastEventID returns other bytes once the connection has received more, because the header is kept as a view of the read buffer (20/20 per native cell) #717 and Context.Detach clones the headers, method, path and raw query but not the route params, query and cookie caches, Host, or middleware-set strings, so a detached Context reads other bytes on epoll and io_uring (7 fields, 20/20) #718 do not record what fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723 finally fixed. Round 2 of fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723 fixed four more defects under the existing issues: the response headers set before
Detachand the request body (both under Context.Detach clones the headers, method, path and raw query but not the route params, query and cookie caches, Host, or middleware-set strings, so a detached Context reads other bytes on epoll and io_uring (7 fields, 20/20) #718), and the requestidEnableStdContextvalue and the otel carrier values (both under middleware/websocket: on epoll and io_uring, Conn.Query returns garbage once a frame has arrived, because the captured query strings alias the engine's read buffer (200/200 per engine) #714). Context.Detach clones the headers, method, path and raw query but not the route params, query and cookie caches, Host, or middleware-set strings, so a detached Context reads other bytes on epoll and io_uring (7 fields, 20/20) #718's title still says "7 fields" and names neither; middleware/websocket: on epoll and io_uring, Conn.Query returns garbage once a frame has arrived, because the captured query strings alias the engine's read buffer (200/200 per engine) #714's title is only about websocketConn.Query, and the requestid and otel changes are per request and affect any code that keepsc.Context(), not only streams. None of the three issues has a comment. Add a comment to middleware/websocket: on epoll and io_uring, Conn.Query returns garbage once a frame has arrived, because the captured query strings alias the engine's read buffer (200/200 per engine) #714 and Context.Detach clones the headers, method, path and raw query but not the route params, query and cookie caches, Host, or middleware-set strings, so a detached Context reads other bytes on epoll and io_uring (7 fields, 20/20) #718 recording the expanded scope with the T/R1/H counts from fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723's Failing-first table, so the closed issues and the release notes carry it.The PR body's ENOMEM sentence is broader than its evidence. fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723's CI section says "No ENOMEM retry fired in this run, the two red runs, or any local run." The counted runs and the three CI Unit jobs are clean (0
server start retriedlines; every io_uring listener reported tier=high), but the server-start ENOMEM retry did fire, 3 to 15 tries, in about 12 superseded local mutant runs. The probe retry (c714ProbeIOUring,detach_capture_alias_linux_test.go:323, and the same helper in the ws, sse and otel tests) never logs, so no log can show whether it fired, locally or in CI. Make the probe retryt.Logfwhen it fires, so CI can show it; the sentence should have been limited to the counted runs.Deliberately red CI runs, for the record. The branch is red-first: CI runs 36320632412 (
9a89e0c) and 36321102421 (50825c1) are red on purpose (the tests before the fixes) and pair with the green run 36321745156 (c98f068). They belong in the maintainers' deliberate-red runs ledger. Because several commits on the branch fail CI on their own, fix: clone the request values a detached stream keeps (websocket Conn.Query, SSE Last-Event-ID, Context.Detach, requestid and otel context values) (#714, #717, #718) #723 must be squash-merged, never rebase-merged.Not carried over: round 2 also asked whether #723's public body should keep the sentence about the multishot receive mode reading another connection's bytes. The maintainer chose public handling of that class (#731, #732, #733), so it stays.