From 8bf6dd28ef54e75c9e6e0ea90547f6d4e5e75795 Mon Sep 17 00:00:00 2001 From: Igor Silvestre Date: Mon, 17 Nov 2025 06:12:40 -0300 Subject: [PATCH 1/9] chore: fix typo in Accept function (#544) --- accept.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accept.go b/accept.go index cc990428..bcfd38e8 100644 --- a/accept.go +++ b/accept.go @@ -90,7 +90,7 @@ func (opts *AcceptOptions) cloneWithDefaults() *AcceptOptions { } // Accept accepts a WebSocket handshake from a client and upgrades the -// the connection to a WebSocket. +// connection to a WebSocket. // // Accept will not allow cross origin requests by default. // See the InsecureSkipVerify and OriginPatterns options to allow cross origin requests. From 8d3545afb3eae1589606d937c4809ff4df769aa6 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 19 Dec 2025 13:44:35 +0000 Subject: [PATCH 2/9] fix: transmit in single frame when compression enabled (#552) Closes https://github.com/coder/websocket/issues/435 When compression was enabled, `Conn.Write` sent messages across many small frames due to the flate library's internal `bufferFlushSize` (240 bytes). Each flush triggered a `writeFrame` call, producing alternating ~236 and 4 byte frames. `Conn.Write` now compresses the entire message into a buffer first, then transmits it as a single frame. Messages below `flateThreshold` bypass compression and are sent uncompressed in a single frame. --- compress_test.go | 250 +++++++++++++++++++++++++++++++++++++++++++++++ write.go | 65 ++++++++++-- 2 files changed, 305 insertions(+), 10 deletions(-) diff --git a/compress_test.go b/compress_test.go index 1964c84f..81adce9a 100644 --- a/compress_test.go +++ b/compress_test.go @@ -3,11 +3,15 @@ package websocket import ( + "bufio" "bytes" "compress/flate" + "context" "io" + "net" "strings" "testing" + "time" "github.com/coder/websocket/internal/test/assert" "github.com/coder/websocket/internal/test/xrand" @@ -59,3 +63,249 @@ func BenchmarkFlateReader(b *testing.B) { io.ReadAll(r) } } + +// TestWriteSingleFrameCompressed verifies that Conn.Write sends compressed +// messages in a single frame instead of multiple frames, and that messages +// below the flateThreshold are sent uncompressed. +// This is a regression test for https://github.com/coder/websocket/issues/435 +func TestWriteSingleFrameCompressed(t *testing.T) { + t.Parallel() + + var ( + flateThreshold = 64 + + largeMsg = []byte(strings.Repeat("hello world ", 100)) + smallMsg = []byte("small message") + ) + + testCases := []struct { + name string + mode CompressionMode + msg []byte + wantRsv1 bool // true = compressed, false = uncompressed + }{ + {"ContextTakeover/AboveThreshold", CompressionContextTakeover, largeMsg, true}, + {"NoContextTakeover/AboveThreshold", CompressionNoContextTakeover, largeMsg, true}, + {"ContextTakeover/BelowThreshold", CompressionContextTakeover, smallMsg, false}, + {"NoContextTakeover/BelowThreshold", CompressionNoContextTakeover, smallMsg, false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + clientConn, serverConn := net.Pipe() + defer clientConn.Close() + defer serverConn.Close() + + c := newConn(connConfig{ + rwc: clientConn, + client: true, + copts: tc.mode.opts(), + flateThreshold: flateThreshold, + br: bufio.NewReader(clientConn), + bw: bufio.NewWriterSize(clientConn, 4096), + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*100) + defer cancel() + + writeDone := make(chan error, 1) + go func() { + writeDone <- c.Write(ctx, MessageText, tc.msg) + }() + + reader := bufio.NewReader(serverConn) + readBuf := make([]byte, 8) + + h, err := readFrameHeader(reader, readBuf) + assert.Success(t, err) + + _, err = io.CopyN(io.Discard, reader, h.payloadLength) + assert.Success(t, err) + + assert.Equal(t, "opcode", opText, h.opcode) + assert.Equal(t, "rsv1 (compressed)", tc.wantRsv1, h.rsv1) + assert.Equal(t, "fin", true, h.fin) + + err = <-writeDone + assert.Success(t, err) + }) + } +} + +// TestWriteThenWriterContextTakeover verifies that using Conn.Write followed by +// Conn.Writer works correctly with context takeover enabled. This tests that +// the flateWriter destination is properly restored after Conn.Write redirects +// it to a temporary buffer. +func TestWriteThenWriterContextTakeover(t *testing.T) { + t.Parallel() + + clientConn, serverConn := net.Pipe() + defer clientConn.Close() + defer serverConn.Close() + + client := newConn(connConfig{ + rwc: clientConn, + client: true, + copts: CompressionContextTakeover.opts(), + flateThreshold: 64, + br: bufio.NewReader(clientConn), + bw: bufio.NewWriterSize(clientConn, 4096), + }) + + server := newConn(connConfig{ + rwc: serverConn, + client: false, + copts: CompressionContextTakeover.opts(), + flateThreshold: 64, + br: bufio.NewReader(serverConn), + bw: bufio.NewWriterSize(serverConn, 4096), + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond*500) + defer cancel() + + msg1 := []byte(strings.Repeat("first message ", 100)) + msg2 := []byte(strings.Repeat("second message ", 100)) + + type readResult struct { + typ MessageType + p []byte + err error + } + readCh := make(chan readResult, 3) + go func() { + for range 3 { + typ, p, err := server.Read(ctx) + readCh <- readResult{typ, p, err} + } + }() + + // We want to verify mixing `Write` and `Writer` usages still work. + // + // To this end, we call them in this order: + // - `Write` + // - `Writer` + // - `Write` + // + // This verifies that it works for a `Write` followed by a `Writer` + // as well as a `Writer` followed by a `Write`. + + // 1. `Write` API + err := client.Write(ctx, MessageText, msg1) + assert.Success(t, err) + + r := <-readCh + assert.Success(t, r.err) + assert.Equal(t, "Write type", MessageText, r.typ) + assert.Equal(t, "Write content", string(msg1), string(r.p)) + + // 2. `Writer` API + w, err := client.Writer(ctx, MessageBinary) + assert.Success(t, err) + _, err = w.Write(msg2) + assert.Success(t, err) + assert.Success(t, w.Close()) + + r = <-readCh + assert.Success(t, r.err) + assert.Equal(t, "Writer type", MessageBinary, r.typ) + assert.Equal(t, "Writer content", string(msg2), string(r.p)) + + // 3. `Write` API again + err = client.Write(ctx, MessageText, msg1) + assert.Success(t, err) + + r = <-readCh + assert.Success(t, r.err) + assert.Equal(t, "Write type", MessageText, r.typ) + assert.Equal(t, "Write content", string(msg1), string(r.p)) +} + +// TestCompressionDictionaryPreserved verifies that context takeover mode +// preserves the compression dictionary across Conn.Write calls, resulting +// in better compression for consecutive similar messages. +func TestCompressionDictionaryPreserved(t *testing.T) { + t.Parallel() + + msg := []byte(strings.Repeat(`{"type":"event","data":"value"}`, 50)) + + takeoverClient, takeoverServer := net.Pipe() + defer takeoverClient.Close() + defer takeoverServer.Close() + + withTakeover := newConn(connConfig{ + rwc: takeoverClient, + client: true, + copts: CompressionContextTakeover.opts(), + flateThreshold: 64, + br: bufio.NewReader(takeoverClient), + bw: bufio.NewWriterSize(takeoverClient, 4096), + }) + + noTakeoverClient, noTakeoverServer := net.Pipe() + defer noTakeoverClient.Close() + defer noTakeoverServer.Close() + + withoutTakeover := newConn(connConfig{ + rwc: noTakeoverClient, + client: true, + copts: CompressionNoContextTakeover.opts(), + flateThreshold: 64, + br: bufio.NewReader(noTakeoverClient), + bw: bufio.NewWriterSize(noTakeoverClient, 4096), + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + + // Capture compressed sizes for both modes + var withTakeoverSizes, withoutTakeoverSizes []int64 + + reader1 := bufio.NewReader(takeoverServer) + reader2 := bufio.NewReader(noTakeoverServer) + readBuf := make([]byte, 8) + + // Send 3 identical messages each + for range 3 { + // With context takeover + writeDone1 := make(chan error, 1) + go func() { + writeDone1 <- withTakeover.Write(ctx, MessageText, msg) + }() + + h1, err := readFrameHeader(reader1, readBuf) + assert.Success(t, err) + + _, err = io.CopyN(io.Discard, reader1, h1.payloadLength) + assert.Success(t, err) + + withTakeoverSizes = append(withTakeoverSizes, h1.payloadLength) + assert.Success(t, <-writeDone1) + + // Without context takeover + writeDone2 := make(chan error, 1) + go func() { + writeDone2 <- withoutTakeover.Write(ctx, MessageText, msg) + }() + + h2, err := readFrameHeader(reader2, readBuf) + assert.Success(t, err) + + _, err = io.CopyN(io.Discard, reader2, h2.payloadLength) + assert.Success(t, err) + + withoutTakeoverSizes = append(withoutTakeoverSizes, h2.payloadLength) + assert.Success(t, <-writeDone2) + } + + // With context takeover, the 2nd and 3rd messages should be smaller than + // without context takeover (dictionary helps compress repeated patterns). + // The first message will be similar size for both modes since there's no + // prior dictionary. But subsequent messages benefit from context takeover. + if withTakeoverSizes[2] >= withoutTakeoverSizes[2] { + t.Errorf("context takeover should compress better: with=%d, without=%d", + withTakeoverSizes[2], withoutTakeoverSizes[2]) + } +} diff --git a/write.go b/write.go index d7172a7b..2e12153b 100644 --- a/write.go +++ b/write.go @@ -14,6 +14,7 @@ import ( "net" "time" + "github.com/coder/websocket/internal/bpool" "github.com/coder/websocket/internal/errd" "github.com/coder/websocket/internal/util" ) @@ -100,23 +101,17 @@ func (c *Conn) writer(ctx context.Context, typ MessageType) (io.WriteCloser, err } func (c *Conn) write(ctx context.Context, typ MessageType, p []byte) (int, error) { - mw, err := c.writer(ctx, typ) + err := c.msgWriter.reset(ctx, typ) if err != nil { return 0, err } + defer c.msgWriter.mu.unlock() - if !c.flate() { - defer c.msgWriter.mu.unlock() + if !c.flate() || len(p) < c.flateThreshold { return c.writeFrame(ctx, true, false, c.msgWriter.opcode, p) } - n, err := mw.Write(p) - if err != nil { - return n, err - } - - err = mw.Close() - return n, err + return c.msgWriter.writeCompressedFrame(ctx, p) } func (mw *msgWriter) reset(ctx context.Context, typ MessageType) error { @@ -142,6 +137,56 @@ func (mw *msgWriter) putFlateWriter() { } } +// writeCompressedFrame compresses and writes p as a single frame. +func (mw *msgWriter) writeCompressedFrame(ctx context.Context, p []byte) (int, error) { + err := mw.writeMu.lock(mw.ctx) + if err != nil { + return 0, fmt.Errorf("failed to write: %w", err) + } + defer mw.writeMu.unlock() + + if mw.closed { + return 0, errors.New("cannot use closed writer") + } + + mw.ensureFlate() + + buf := bpool.Get() + defer bpool.Put(buf) + + // Buffer compressed output so we can write as + // a single frame instead of chunked frames. + origWriter := mw.trimWriter.w + mw.trimWriter.w = buf + defer func() { + mw.trimWriter.w = origWriter + }() + + _, err = mw.flateWriter.Write(p) + if err != nil { + return 0, fmt.Errorf("failed to compress: %w", err) + } + + err = mw.flateWriter.Flush() + if err != nil { + return 0, fmt.Errorf("failed to flush compression: %w", err) + } + + mw.trimWriter.reset() + + if !mw.flateContextTakeover() { + mw.putFlateWriter() + } + + mw.closed = true + + _, err = mw.c.writeFrame(ctx, true, true, mw.opcode, buf.Bytes()) + if err != nil { + return 0, err + } + return len(p), nil +} + // Write writes the given bytes to the WebSocket connection. func (mw *msgWriter) Write(p []byte) (_ int, err error) { err = mw.writeMu.lock(mw.ctx) From 9f473adfa59519d1e0295cd7a988d965085b701a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Jan 2026 14:58:02 +0200 Subject: [PATCH 3/9] docs: add document for AI agents (AGENTS.md) (#547) --- .gitignore | 1 + AGENTS.md | 157 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 .gitignore create mode 100644 AGENTS.md diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..d1941b1c --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +PLAN.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..93bf8d64 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,157 @@ +# Agents + +This document helps AI agents work effectively in this codebase. It explains the philosophy, patterns, and pitfalls behind the code, so you can make good decisions on any task, not just scenarios explicitly covered. + +## Philosophy + +This is a minimal, idiomatic WebSocket library. Simplicity is a feature. + +Before adding code, articulate why it's necessary in one sentence. If you cannot justify the addition, it probably isn't needed. + +Before adding a dependency, don't. Tests requiring external packages are isolated in `internal/thirdparty`. + +## Workflow + +Every task follows phases. Do not skip phases; if you realize you missed something, return to the appropriate phase. + +**Making changes:** + +1. **Research** - Understand the problem and codebase before acting +2. **Plan** - Articulate your approach before implementing (when asked, or for complex changes) +3. **Implement** - Make changes in small, verifiable steps +4. **Verify** - Confirm correctness using external tools + +For trivial changes (typo fixes, comment updates), an abbreviated workflow is acceptable. + +## Research + +Research in sequential passes. Each pass has one focus. Don't skip ahead to code until you've completed the earlier passes. + +**Pass 1: Read the issue.** If you were given a link, read it now. Do not explore code, do not pass go. Fetch the linked issue or document first. Summarize what it asks for. If you cannot restate the problem, you are not ready to proceed. + +**Pass 2: Read linked references.** Follow every link in the issue: related issues, RFCs, external docs. Document what you learn. Code comments reference RFC 6455 (WebSocket), RFC 7692 (compression), and RFC 8441 (HTTP/2). + +**Pass 3: Trace the code.** Start from public API inward: `Accept` (server) or `Dial` (client) → `Conn` → `Reader`/`Writer`. Read tests for intent; the autobahn-testsuite (`autobahn_test.go`) validates protocol compliance. + +**Pass 4: Check both platforms.** Native Go files have `//go:build !js`. WASM lives in `ws_js.go`. Same API, different implementations. WASM wraps browser APIs and cannot control framing, compression, or masking. + +**Pass 5: Search exhaustively.** If the change affects a pattern used in multiple places, grep for all instances. Missing one creates inconsistent behavior. + +**Pass 6: Document unknowns.** List what you still don't know. Unknown unknowns become known unknowns when you ask "what am I still unsure about?" + +**After all passes:** Can you restate the problem in your own words? If not, return to Pass 1. Gaps in earlier passes will cause problems later. + +## Plan + +When asked to plan, write to `PLAN.md`. Write for someone else, not yourself; don't skip context you already know. + +**Pass 1: Document research.** Summarize what you learned in Research. Follow every reference; document findings so the implementer can verify. If the research section is empty, you haven't researched enough. + +**Pass 2: Consider approaches.** For non-trivial problems, enumerate at least two approaches. For each, note: what would change, what could go wrong, what's the tradeoff. + +**Pass 3: Detail the chosen approach.** Explain what and why, not step-by-step how. Point to specific files, functions, line numbers. Make claims verifiable. Leave room for the implementer to find a better solution. + +**Pass 4: List open questions.** What's still unclear? What assumptions are you making? What would change your approach? + +**After all passes:** Review from the implementer's perspective. Could they start work with only this document? If not, add what's missing. + +## Implement + +Implement in sequential passes. Don't write code until you've completed the verification passes. + +**Pass 1: Verify understanding.** Did you do your research? Can you state your approach in one sentence? If requirements are ambiguous, stop and ask. A wrong assumption wastes more time than a quick question. + +**Pass 2: Check scope.** Does this need to exist? Check if it already exists in the API. Is this the library's job or the user's job? The library handles protocol correctness; application concerns (reconnection, auth, routing) belong in user code. + +**Pass 3: Check invariants.** Walk through Key Invariants before writing code: +- Reads: Will something still read from the connection? +- Pools: Will pooled objects be returned on all paths? +- Locks: Are you using context-aware `mu`, not `sync.Mutex`? +- Independence: Are you coupling reads and writes unnecessarily? + +**Pass 4: Implement.** Make the change. Every change needs a reason; if you can't articulate why it improves things, don't make it. Preserve existing comments unless you can prove they're wrong. + +**Pass 5: Verify examples.** Trace through usage examples as if writing real code. If an example wouldn't compile, the design is wrong. Check edge cases: what happens on error? On cancellation? + +**After all passes:** If feedback identifies a problem, fix that specific problem. Don't pivot to a new approach without articulating what failed and why the new approach avoids it. + +## Verify + +Verify using external signals. Self-assessment is unreliable; these tools are the ground truth. + +**Pass 1: Run tests.** `go test ./...` must pass. If tests fail, return to Research to understand why, not to Implement to "fix" it. + +**Pass 2: Run vet.** `go vet ./...` must report no issues. + +**Pass 3: Check platforms.** Code must compile on both native Go and WASM. API changes need both implementations; test that method signatures match. + +**Pass 4: Protocol compliance.** For protocol changes, run `AUTOBAHN=1 go test`. Compare wire bytes when implementations disagree; check against the RFC. + +**After all passes:** If any pass fails, fix the issue and restart from Pass 1. Don't consider a change complete until all passes succeed. + +## When Uncertain + +If confidence is low or requirements are ambiguous: + +1. State what you're uncertain about +2. Identify what information would resolve the uncertainty +3. Gather that information or ask for clarification + +Do not proceed with low-confidence assumptions. + +For complex changes, approach from multiple angles. If two approaches give different answers, that discrepancy demands resolution before proceeding. + +## Code Style + +Follow Go conventions: [Effective Go](https://go.dev/doc/effective_go), [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments), and [Go Proverbs](https://go-proverbs.github.io/). Be concise, declarative, and factual. + +Never use emdash. Use commas, semicolons, or separate sentences. + +**Doc comments** start with the name and state what it does. Structure: `// [Name] [verb]s [what]. [Optional: when/why to use it].` Put useful information where users make decisions (usually the constructor, not methods). + +**Inline comments** are terse. Prefer end-of-line when short enough. + +**Explain why, not what.** The code shows what it does; comments should explain reasoning, non-obvious decisions, or edge cases. + +**Wrap comments** at ~80 characters, continuing naturally at word boundaries. + +**Naming matters.** Before proposing a name, stop and review existing names in the file. Ask: what would someone assume from this name? Does it fit with how similar things are named? A good name is accurate on its own and consistent in context. + +**Comment content:** +- Add information beyond what the code shows (not tautologies) +- State directly: "Returns X" (not "Note that this returns X") +- Drop filler: "basically", "actually", "really" add nothing + +## Key Invariants + +**Always read from connections.** Control frames (ping, pong, close) arrive on the read path. A connection that never reads will miss them and misbehave. `CloseRead` exists for write-only patterns. + +**Pooled objects must be returned.** `flate.Writer` is ~1.2MB. Leaking them causes memory growth. Follow `get*()` / `put*()` patterns; return on all paths including errors. + +**Locks must respect context.** The `mu` type in `conn.go` unblocks when context cancels or connection closes. Using `sync.Mutex` for user-facing operations would block forever on stuck connections. + +**Reads and writes are independent.** They have separate locks (`readMu`, `writeFrameMu`) and can happen concurrently. Keep them decoupled. + +**Masking is asymmetric.** Clients mask payloads; servers don't. The mask is applied into the bufio.Writer buffer, not in-place on source bytes. + +## Commits and PRs + +Use conventional commits: `type(scope): description`. Scope is optional but include it when changes are constrained to a single file or directory. + +**Choose precise verbs:** `add` (new functionality), `fix` (broken behavior), `prevent` (undesirable state), `allow` (enable action), `handle` (edge cases), `improve` (performance/quality), `use` (change approach), `skip` (bypass), `remove` (delete), `rewrite` (substantial rework). + +**Before writing the commit message**, articulate what was wrong with the previous behavior and why this change fixes it. The commit message captures this reasoning. + +**Commit messages explain why.** State what was wrong, why it mattered, and why this solution was chosen. One to three sentences. Keep the tone neutral. + +**PR descriptions are for humans.** Explain what reviewers can't see: the why, alternatives considered, and tradeoffs made. Show evidence (logs, benchmarks, screenshots). Be explicit about what the PR doesn't do. + +**Link issues:** `Fixes #123` (PR resolves it), `Refs #123` (related), `Updates #123` (partial progress). + +## RFC References + +- RFC 6455: The WebSocket Protocol +- RFC 7692: Compression Extensions for WebSocket (permessage-deflate) +- RFC 8441: Bootstrapping WebSockets with HTTP/2 + +Section numbers in code comments refer to these RFCs. From 39b7b07a7a6e733c564d43921fc1b7ea16370b5b Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 23 Feb 2026 03:17:10 -0500 Subject: [PATCH 4/9] docs: remove mention of resolved issue in README (#556) --- README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/README.md b/README.md index 6e986897..72afa535 100644 --- a/README.md +++ b/README.md @@ -109,8 +109,6 @@ Advantages of [gorilla/websocket](https://github.com/gorilla/websocket): - Mature and widely used - [Prepared writes](https://pkg.go.dev/github.com/gorilla/websocket#PreparedMessage) - Configurable [buffer sizes](https://pkg.go.dev/github.com/gorilla/websocket#hdr-Buffers) -- No extra goroutine per connection to support cancellation with context.Context. This costs github.com/coder/websocket 2 KB of memory per connection. - - Will be removed soon with [context.AfterFunc](https://github.com/golang/go/issues/57928). See [#411](https://github.com/nhooyr/websocket/issues/411) Advantages of github.com/coder/websocket: From c98e9dc2d6f69af6172dd1ed212c48b35fc54f22 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 23 Feb 2026 10:43:51 +0200 Subject: [PATCH 5/9] docs: fix lint issue (#557) --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 93bf8d64..711bc96a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,6 +64,7 @@ Implement in sequential passes. Don't write code until you've completed the veri **Pass 2: Check scope.** Does this need to exist? Check if it already exists in the API. Is this the library's job or the user's job? The library handles protocol correctness; application concerns (reconnection, auth, routing) belong in user code. **Pass 3: Check invariants.** Walk through Key Invariants before writing code: + - Reads: Will something still read from the connection? - Pools: Will pooled objects be returned on all paths? - Locks: Are you using context-aware `mu`, not `sync.Mutex`? @@ -118,6 +119,7 @@ Never use emdash. Use commas, semicolons, or separate sentences. **Naming matters.** Before proposing a name, stop and review existing names in the file. Ask: what would someone assume from this name? Does it fit with how similar things are named? A good name is accurate on its own and consistent in context. **Comment content:** + - Add information beyond what the code shows (not tautologies) - State directly: "Returns X" (not "Note that this returns X") - Drop filler: "basically", "actually", "really" add nothing From d099e1621e8d0f080c9f3b87c5a8587e0b722723 Mon Sep 17 00:00:00 2001 From: Goyabean <118635203+GeObts@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:25:38 -0600 Subject: [PATCH 6/9] docs: fix roadmap links (#558) --- README.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 72afa535..3f174d5b 100644 --- a/README.md +++ b/README.md @@ -36,15 +36,15 @@ go get github.com/coder/websocket See GitHub issues for minor issues but the major future enhancements are: -- [ ] Perfect examples [#217](https://github.com/nhooyr/websocket/issues/217) -- [ ] wstest.Pipe for in memory testing [#340](https://github.com/nhooyr/websocket/issues/340) -- [ ] Ping pong heartbeat helper [#267](https://github.com/nhooyr/websocket/issues/267) -- [ ] Ping pong instrumentation callbacks [#246](https://github.com/nhooyr/websocket/issues/246) -- [ ] Graceful shutdown helpers [#209](https://github.com/nhooyr/websocket/issues/209) -- [ ] Assembly for WebSocket masking [#16](https://github.com/nhooyr/websocket/issues/16) - - WIP at [#326](https://github.com/nhooyr/websocket/pull/326), about 3x faster -- [ ] HTTP/2 [#4](https://github.com/nhooyr/websocket/issues/4) -- [ ] The holy grail [#402](https://github.com/nhooyr/websocket/issues/402) +- [ ] Perfect examples [#217](https://github.com/coder/websocket/issues/217) +- [ ] wstest.Pipe for in memory testing [#340](https://github.com/coder/websocket/issues/340) +- [ ] Ping pong heartbeat helper [#267](https://github.com/coder/websocket/issues/267) +- [ ] Ping pong instrumentation callbacks [#246](https://github.com/coder/websocket/issues/246) +- [ ] Graceful shutdown helpers [#209](https://github.com/coder/websocket/issues/209) +- [ ] Assembly for WebSocket masking [#16](https://github.com/coder/websocket/issues/16) + - WIP at [#326](https://github.com/coder/websocket/pull/326), about 3x faster +- [ ] HTTP/2 [#4](https://github.com/coder/websocket/issues/4) +- [ ] The holy grail [#402](https://github.com/coder/websocket/issues/402) ## Examples @@ -126,9 +126,9 @@ Advantages of github.com/coder/websocket: - Gorilla requires registering a pong callback before sending a Ping - Can target Wasm ([gorilla/websocket#432](https://github.com/gorilla/websocket/issues/432)) - Transparent message buffer reuse with [wsjson](https://pkg.go.dev/github.com/coder/websocket/wsjson) subpackage -- [1.75x](https://github.com/nhooyr/websocket/releases/tag/v1.7.4) faster WebSocket masking implementation in pure Go +- [1.75x](https://github.com/coder/websocket/releases/tag/v1.7.4) faster WebSocket masking implementation in pure Go - Gorilla's implementation is slower and uses [unsafe](https://golang.org/pkg/unsafe/). - Soon we'll have assembly and be 3x faster [#326](https://github.com/nhooyr/websocket/pull/326) + Soon we'll have assembly and be 3x faster [#326](https://github.com/coder/websocket/pull/326) - Full [permessage-deflate](https://tools.ietf.org/html/rfc7692) compression extension support - Gorilla only supports no context takeover mode - [CloseRead](https://pkg.go.dev/github.com/coder/websocket#Conn.CloseRead) helper for write only connections ([gorilla/websocket#492](https://github.com/gorilla/websocket/issues/492)) From 70393646f4d7f38613019f50c3dc78cead816191 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 14 Jun 2026 20:00:02 -0700 Subject: [PATCH 7/9] avoid per-frame cleanup closures on read to remove 2 allocs per frame read (#565) read: avoid per-frame cleanup closures Every frame header and payload read called prepareRead, which returned a cleanup function to clear the timeout and translate close or cancellation errors. That function captured the context, connection, and the address of the caller's named error result. Because prepareRead returned the function, the closure outlived its stack frame and Go allocated its captured state on the heap for every frame read. Move the cleanup logic to a normal finishRead method and defer a direct method call instead. This preserves timeout cleanup and error translation without returning a closure. Compiler escape analysis with -gcflags=-m=2 confirms that the old function literal escaped and forced the named error result in readFrameHeader and readFramePayload onto the heap; neither escape remains after this change. The results below compare parent d099e162 with this commit on an Apple M4 Max using GOMAXPROCS=1 and benchstat over 10 samples. A temporary in-package harness, not included in this commit, repeatedly called one internal frame read. Header reads parse a minimal frame header; payload reads copy 512 bytes from a buffered repeating reader. The background cases use context.Background, while the cancelable cases use an uncanceled context.WithCancel. Removing the escaping closure eliminates 2 allocations and 64 bytes from every frame read: one allocation for the closure environment and one for the named error result retained by that closure. Header reads with a background context improve from 170.5 to 118.5 ns/op, 216 to 152 B/op, and 6 to 4 allocs/op. Header reads with a cancelable context improve from 211.1 to 158.7 ns/op with the same allocation reduction. Payload reads remove the same fixed overhead; they remain slower because the benchmark also copies 512 bytes. Both context types benefit because this commit does not change timeout registration. It only removes cleanup allocations made after every prepareRead call. An interleaved 12-sample BenchmarkConn/disabledCompress run, which exercises complete message reads, improves from 3.875 to 3.657 us/op (-5.63%), 42 to 32 allocs/op (-23.81%), and 1536 to 1216 B/op (-20.83%). --- read.go | 55 ++++++++++++++++++++++++++----------------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/read.go b/read.go index 64822511..871b1105 100644 --- a/read.go +++ b/read.go @@ -217,51 +217,48 @@ func (c *Conn) readLoop(ctx context.Context) (header, error) { } } -// prepareRead sets the readTimeout context and returns a done function -// to be called after the read is done. It also returns an error if the -// connection is closed. The reference to the error is used to assign -// an error depending on if the connection closed or the context timed -// out during use. Typically, the referenced error is a named return -// variable of the function calling this method. -func (c *Conn) prepareRead(ctx context.Context, err *error) (func(), error) { +// prepareRead sets the read timeout and checks whether the connection is closed. +func (c *Conn) prepareRead(ctx context.Context) error { select { case <-c.closed: - return nil, net.ErrClosed + return net.ErrClosed default: } c.setupReadTimeout(ctx) - done := func() { - c.clearReadTimeout() - select { - case <-c.closed: - if *err != nil { - *err = net.ErrClosed - } - default: - } - if *err != nil && ctx.Err() != nil { - *err = ctx.Err() - } - } - c.closeStateMu.Lock() closeReceivedErr := c.closeReceivedErr c.closeStateMu.Unlock() if closeReceivedErr != nil { - defer done() - return nil, closeReceivedErr + c.clearReadTimeout() + return closeReceivedErr } - return done, nil + return nil +} + +// finishRead clears the read timeout and reports whether the connection or +// operation context ended while the read was in progress. +func (c *Conn) finishRead(ctx context.Context, err *error) { + c.clearReadTimeout() + select { + case <-c.closed: + if *err != nil { + *err = net.ErrClosed + } + default: + } + if *err != nil && ctx.Err() != nil { + *err = ctx.Err() + } } func (c *Conn) readFrameHeader(ctx context.Context) (_ header, err error) { - readDone, err := c.prepareRead(ctx, &err) + err = c.prepareRead(ctx) if err != nil { return header{}, err } - defer readDone() + defer c.finishRead(ctx, &err) h, err := readFrameHeader(c.br, c.readHeaderBuf[:]) if err != nil { @@ -272,11 +269,11 @@ func (c *Conn) readFrameHeader(ctx context.Context) (_ header, err error) { } func (c *Conn) readFramePayload(ctx context.Context, p []byte) (_ int, err error) { - readDone, err := c.prepareRead(ctx, &err) + err = c.prepareRead(ctx) if err != nil { return 0, err } - defer readDone() + defer c.finishRead(ctx, &err) n, err := io.ReadFull(c.br, p) if err != nil { From 9c8faadccd1b679e811a79ce506f8a10237251ad Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 14 Jun 2026 20:25:28 -0700 Subject: [PATCH 8/9] conn: skip timeout callbacks for background contexts (#566) Every frame read and write registered a context.AfterFunc callback so a context cancellation could close the connection. context.Background cannot be canceled because its Done channel is nil, but the code still constructed, stored, stopped, and cleared a callback registration for each operation. Skip that work when ctx.Done() is nil, and only clear a timeout when one was actually installed. BenchmarkConn also joins its writer goroutine after timing so repeated benchmark runs finish cleanly. The results below compare parent 70393646 with this commit on an Apple M4 Max using GOMAXPROCS=1 over 10 samples. A temporary in-package harness, not included in this commit, repeatedly called one internal frame read using either context.Background or an uncanceled context.WithCancel. Header reads parse a minimal frame header; payload reads copy 512 bytes from a buffered repeating reader. Before this change, setupReadTimeout called context.AfterFunc even for context.Background. Avoiding that dead registration reduces background header reads from 121.95 to 29.94 ns/op and payload reads from 124.17 to 29.15 ns/op. Both fall from 152 B/op and 4 allocs/op to zero. Cancelable contexts still need an AfterFunc registration, so they remain at 152 B/op and 4 allocs/op. BenchmarkConn/disabledCompress uses a cancelable context as well, so it remains at 1219 B/op and 32 allocs/op. --- conn.go | 14 ++++++++++++-- conn_test.go | 10 +++++++++- read.go | 28 ++++++++++++++++------------ write.go | 5 +++-- 4 files changed, 40 insertions(+), 17 deletions(-) diff --git a/conn.go b/conn.go index 09234871..bfc78aaa 100644 --- a/conn.go +++ b/conn.go @@ -168,24 +168,34 @@ func (c *Conn) close() error { return err } -func (c *Conn) setupWriteTimeout(ctx context.Context) { +func (c *Conn) setupWriteTimeout(ctx context.Context) bool { + if ctx.Done() == nil { + return false + } + stop := context.AfterFunc(ctx, func() { c.clearWriteTimeout() c.close() }) swapTimeoutStop(&c.writeTimeoutStop, &stop) + return true } func (c *Conn) clearWriteTimeout() { swapTimeoutStop(&c.writeTimeoutStop, nil) } -func (c *Conn) setupReadTimeout(ctx context.Context) { +func (c *Conn) setupReadTimeout(ctx context.Context) bool { + if ctx.Done() == nil { + return false + } + stop := context.AfterFunc(ctx, func() { c.clearReadTimeout() c.close() }) swapTimeoutStop(&c.readTimeoutStop, &stop) + return true } func (c *Conn) clearReadTimeout() { diff --git a/conn_test.go b/conn_test.go index c3ccc886..01af8cd4 100644 --- a/conn_test.go +++ b/conn_test.go @@ -592,10 +592,11 @@ func BenchmarkConn(b *testing.B) { msg := []byte(strings.Repeat("1234", 128)) readBuf := make([]byte, len(msg)) writes := make(chan struct{}) - defer close(writes) werrs := make(chan error) + writerDone := make(chan struct{}) go func() { + defer close(writerDone) for range writes { select { case werrs <- c1.Write(bb.ctx, websocket.MessageText, msg): @@ -650,6 +651,13 @@ func BenchmarkConn(b *testing.B) { } b.StopTimer() + close(writes) + select { + case <-writerDone: + case <-bb.ctx.Done(): + b.Fatal(bb.ctx.Err()) + } + b.ReportMetric(float64(*bytesWritten/b.N), "written/op") b.ReportMetric(float64(*bytesRead/b.N), "read/op") diff --git a/read.go b/read.go index 871b1105..c19f41de 100644 --- a/read.go +++ b/read.go @@ -218,29 +218,33 @@ func (c *Conn) readLoop(ctx context.Context) (header, error) { } // prepareRead sets the read timeout and checks whether the connection is closed. -func (c *Conn) prepareRead(ctx context.Context) error { +func (c *Conn) prepareRead(ctx context.Context) (bool, error) { select { case <-c.closed: - return net.ErrClosed + return false, net.ErrClosed default: } - c.setupReadTimeout(ctx) + timeoutSet := c.setupReadTimeout(ctx) c.closeStateMu.Lock() closeReceivedErr := c.closeReceivedErr c.closeStateMu.Unlock() if closeReceivedErr != nil { - c.clearReadTimeout() - return closeReceivedErr + if timeoutSet { + c.clearReadTimeout() + } + return false, closeReceivedErr } - return nil + return timeoutSet, nil } // finishRead clears the read timeout and reports whether the connection or // operation context ended while the read was in progress. -func (c *Conn) finishRead(ctx context.Context, err *error) { - c.clearReadTimeout() +func (c *Conn) finishRead(ctx context.Context, err *error, timeoutSet bool) { + if timeoutSet { + c.clearReadTimeout() + } select { case <-c.closed: if *err != nil { @@ -254,11 +258,11 @@ func (c *Conn) finishRead(ctx context.Context, err *error) { } func (c *Conn) readFrameHeader(ctx context.Context) (_ header, err error) { - err = c.prepareRead(ctx) + timeoutSet, err := c.prepareRead(ctx) if err != nil { return header{}, err } - defer c.finishRead(ctx, &err) + defer c.finishRead(ctx, &err, timeoutSet) h, err := readFrameHeader(c.br, c.readHeaderBuf[:]) if err != nil { @@ -269,11 +273,11 @@ func (c *Conn) readFrameHeader(ctx context.Context) (_ header, err error) { } func (c *Conn) readFramePayload(ctx context.Context, p []byte) (_ int, err error) { - err = c.prepareRead(ctx) + timeoutSet, err := c.prepareRead(ctx) if err != nil { return 0, err } - defer c.finishRead(ctx, &err) + defer c.finishRead(ctx, &err, timeoutSet) n, err := io.ReadFull(c.br, p) if err != nil { diff --git a/write.go b/write.go index 2e12153b..81bab4fa 100644 --- a/write.go +++ b/write.go @@ -318,8 +318,9 @@ func (c *Conn) writeFrame(ctx context.Context, fin bool, flate bool, opcode opco return 0, net.ErrClosed default: } - c.setupWriteTimeout(ctx) - defer c.clearWriteTimeout() + if c.setupWriteTimeout(ctx) { + defer c.clearWriteTimeout() + } c.writeHeader.fin = fin c.writeHeader.opcode = opcode From f852351b4f478daa29c4ff33c2f948ab86f804b2 Mon Sep 17 00:00:00 2001 From: Sushant Shankar Date: Thu, 22 Oct 2026 17:51:35 +0530 Subject: [PATCH 9/9] fix: remove racy Flush from h2ServerStream.Close Close can run on a context.AfterFunc goroutine (e.g. after the peer sends RST_STREAM) while the handler goroutine is in Write. Both touched the same http.ResponseWriter, which is not safe for concurrent use. Write already flushes after every write, so the Flush in Close was unnecessary. --- http2.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/http2.go b/http2.go index fa5d4046..1dfae584 100644 --- a/http2.go +++ b/http2.go @@ -42,8 +42,15 @@ func (s *h2ServerStream) Flush() error { func (s *h2ServerStream) Close() error { // TODO(mafredri): Verify if the flush is necessary before closing the reader. - err := s.Flush() - return errors.Join(err, s.ReadCloser.Close()) + + //err := s.Flush() + //return errors.Join(err, s.ReadCloser.Close()) + // Write flushes after every write, so nothing is left buffered here. + // Flushing in Close is also unsafe: Close can run on a context.AfterFunc + // goroutine (e.g. after the peer sends RST_STREAM) while another + // goroutine is in Write, and http.ResponseWriter is not safe for + // concurrent use. + return s.ReadCloser.Close() } // h2ClientStream is a minimal io.ReadWriteCloser for an HTTP/2 extended CONNECT