Add (eql :closed) no-op method on apply-stream-priority#12
Closed
jcmallery wants to merge 33 commits into
Closed
Conversation
…nd regions Global buffer pool for octet buffers with three size classes: Small (16B, max 64 free), Medium (1KB, max 32), Large (16KB, max 32). Lock-free via CAS (Treiber stack): sb-ext:cas on SBCL, sys:compare-and-swap on LW, generic lock fallback. allocate-buffer returns fill-pointer arrays with fp = requested size. (length buffer) returns the requested size; array-total-size returns the size-class capacity. deallocate-buffer resets fp before pooling. with-resource-usage-region: scope-based deallocation for buffers that escape local management (e.g., HEADERS continuation closures). region-track-buffer transfers ownership to the region. Measured on SBCL (50 H2 GET + 10 POST, Apple M3): Small: 2 allocated, 2402 recycled (1201:1) Medium: 5 allocated, 1419 recycled (284:1) Large: 1 allocated, 9 recycled (9:1)
- Add to .asd and package.lisp - Move documentation to mgl-pax. - Align file and package name - Use buffer-pool package from core
HTTP-STREAM-ERROR now derives from ERROR. There is a specific subclass to be raised when we receive the stream error from the peer, as opposed from when we detect it.
New generic function QUEUE-FRAME-REGION to write only part of the provided data. Falls back to QUEUE-FRAME. WRITE-VECTOR-FRAME can be now used to send prepared data vector without copying it, providing QUEUE-FRAME-REGION is specialized for the connection to avoid copying. QUEUE-FRAME-REGION uses WRITE-VECTOR-FRAME now.
There were two functions with same interpretation and it did not work.
Relocate the method, scheme, authority, path slots from server-stream up to the http2-stream common superclass. Both server-stream and client-stream instances now inherit the slots (and their accessor generic functions). Rationale: log-closed-stream in core/frames/headers.lisp calls (get-path stream), (get-method stream) etc. on any h2-stream during http-stream-error handling. The slots were only on server-stream, so these calls signalled no-applicable-method-error on client streams, killing client reactors. Moving to http2-stream keeps the existing server-stream API (all slots still accessible via the same accessor GFs) while making the accessors work uniformly on client streams for diagnostics and debugging. Backward-compatible: server-stream instances retain all slots via inheritance, and no existing code paths change.
…aders Add request pseudo-header slots to client-stream Should fix LOG-CLOSED-STREAM on client streams (presently raises error)
Frame parsing, HPACK encoding/decoding, stream management, and connection processing had no optimization declarations. Default optimization runs generic dispatch, type checks, and no inlining on the H2 framing hot path. 13 core files: classes, frames (main + 6 frame types), hpack, stream-based-connections, payload-streams, pipe, utils. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The mode line attribute said Package: http2/resources but the in-package form uses http2/buffer-pool. Genera reads the mode line to set the package, so the mismatch causes compilation in the wrong package.
FIND-JUST-STREAM-BY-ID returns the keyword :CLOSED when the stream-id is absent from the streams table (the stream was already closed and reaped). APPLY-STREAM-PRIORITY then ran the default method, which calls (SETF (GET-WEIGHT stream) WEIGHT) and (SETF (GET-DEPENDS-ON stream) ...) -- neither has a method specialised on a keyword. Symptom in production: a PRIORITY-bearing frame (or a HEADERS frame with the priority flag) for a closed stream made the H2 writer thread die with SIMPLE-ERROR -- No applicable methods for #<STANDARD-GENERIC-FUNCTION (SETF GET-WEIGHT) ...> with args (WEIGHT :CLOSED) RFC 9113 sec 5.5 explicitly permits PRIORITY frames on closed streams, and RFC 9218 deprecates stream priority entirely, so the right behaviour is "do nothing" -- there is no per-stream state to update. The library already follows this pattern for the three other generics that FIND-JUST-STREAM-BY-ID's keyword return value reaches: - update-window-size (eql :closed) - peer-resets-stream (eql :closed) - get-peer-window-size (eql :closed) apply-stream-priority was missing one. Adding it fixes the crash without changing behaviour on live streams.
Owner
|
Pushed to v2.0.5 branch. Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FIND-JUST-STREAM-BY-ID(core/frames/http2-stream.lisp) returns thekeyword
:CLOSEDwhen the stream-id is absent from the streams table --the stream was already closed and reaped. Then
APPLY-STREAM-PRIORITYruns the default method, which calls
(SETF (GET-WEIGHT stream) WEIGHT)and
(SETF (GET-DEPENDS-ON stream) ...). Neither has a methodspecialised on a keyword, so the writer thread crashes with:
This is reachable in two ways:
PRIORITYframe arrives on a stream-id whose entry has aged out of(get-streams connection).HEADERSframe with thePRIORITYflag set arrives viaparse-header-frame*afterfind-just-stream-by-idreturned:closed.Hit in a production CL-HTTP/SBCL deployment after a long-running
session.
Fix
Add an
(eql :closed)no-op method onapply-stream-priority,mirroring the existing
(eql :closed)no-op methods already presenton the three other generics that
find-just-stream-by-id's keywordreturn value reaches:
update-window-sizecore/frames/data.lisp:226peer-resets-streamcore/frames/rst-and-goaway.lisp:10get-peer-window-sizecore/frames/data.lisp:362apply-stream-priorityRFC 9113 sec 5.5 explicitly permits PRIORITY frames on closed streams,
and RFC 9218 deprecates stream priority entirely, so a no-op is the
correct semantics. The default method's docstring already acknowledges
this: "Does nothing, as priorities are deprecated in RFC9113 anyway."
Verification
Direct call against a closed-stream sentinel now returns NIL instead of
signalling:
11 lines of source change; no behaviour change on live stream objects.