Conversation
The pinned golangci-lint v2.1.6 (built with Go 1.24) panics when analyzing code with the Go 1.26 toolchain installed by setup-go, since go/types refuses to type-check files requiring a newer Go version than the one golangci-lint itself was built with: panic: file requires newer Go version go1.26 (application built with go1.24) Bumped all 4 workflow files that pin golangci-lint (test.yml, test_gc_opt.yml, test_poll_opt.yml, test_poll_opt_gc_opt.yml) to v2.12.2, which is built with Go 1.26.2. Verified locally: golangci-lint v2.12.2 run with the same linters/args as CI (-E gocritic -E misspell -E revive -E godot, with/without --timeout 5m) reports 0 issues on the repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…fined context (#760) Add SafeContext() and SetSafeContext(ctx any) to the Conn interface, backed by atomic.Pointer[any], allowing goroutines other than the owning event-loop to safely read/write a connection's user-defined context without racing against Context()/SetContext(), which remain event-loop-only. - Implement safeCtx atomic.Pointer[any] field and accessors in connection_unix.go/connection_windows.go; wire up initial context on Windows via SetSafeContext at connection construction; reset safeCtx to nil in release(). - Clarify Conn interface doc comments for SafeContext/SetSafeContext (reference Context/SetContext directly) and for LocalAddr/RemoteAddr (note callers must copy the returned net.Addr via String() if used outside EventHandler). - Add TestSafeContext (gnet_test.go) and TestClientSafeContext (client_test.go) covering: nil before first SetSafeContext, rotating concrete types (including nil) via OnTraffic, concurrent SafeContext reads from a background goroutine, no interference with Context()/SetContext(), correctness immediately after Dial/OnOpen, safety when called from OnClose, and reset to nil after release(). - Avoid port 10000 in existing tests (conflicts with a common local service on some machines), switching to 19999. --------- Co-authored-by: Andy Pan <i@andypan.me> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a concurrency-safe connection context API (SafeContext / SetSafeContext) across the core Conn implementations, adds coverage to validate the new contract (including cross-goroutine access and type mixing), and updates CI linting to a newer golangci-lint release.
Changes:
- Extend
ConnwithSafeContext()/SetSafeContext(any)and clarifyLocalAddr/RemoteAddrconcurrency notes. - Implement
safeCtxstorage using atomics in Unix/Windows connection implementations and initialize it for enrolled/client connections. - Add server/client tests for
SafeContextbehavior and bump several hardcoded test ports; update GitHub Actions lint workflows to golangci-lintv2.12.2.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
gnet.go |
Adds SafeContext/SetSafeContext to the public Conn interface and updates address concurrency documentation. |
eventloop_unix.go |
Ensures enrolled connections have both Context and SafeContext initialized from enrollment ctx. |
client_unix.go |
Ensures client-enrolled connections have both Context and SafeContext initialized from dial/enroll ctx. |
connection_unix.go |
Adds atomic-backed safeCtx field and implements SafeContext/SetSafeContext; clears on release(). |
connection_windows.go |
Adds atomic-backed safeCtx field, initializes it in constructors, implements SafeContext/SetSafeContext, clears on release(). |
gnet_test.go |
Updates some fixed ports and adds a TestSafeContext validating concurrency-safe behavior. |
client_test.go |
Adds TestClientSafeContext validating dial-time ctx propagation and concurrency-safe behavior. |
.github/workflows/test.yml |
Bumps golangci-lint version used by the lint job. |
.github/workflows/test_poll_opt.yml |
Bumps golangci-lint version used by the lint job. |
.github/workflows/test_poll_opt_gc_opt.yml |
Bumps golangci-lint version used by the lint job. |
.github/workflows/test_gc_opt.yml |
Bumps golangci-lint version used by the lint job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+433
to
+436
| // SafeContext is the concurrency-safe version of Context that can | ||
| // be invoked on any goroutine. | ||
| SafeContext() (ctx any) | ||
|
|
Comment on lines
+2789
to
+2808
| err := goPool.DefaultWorkerPool.Submit(func() { | ||
| for { | ||
| select { | ||
| case <-s.stopBackground: | ||
| return | ||
| default: | ||
| } | ||
| switch v := c.SafeContext().(type) { | ||
| case nil: | ||
| case *safeCtxPayloadA: | ||
| _ = v.n | ||
| case *safeCtxPayloadB: | ||
| _ = v.s | ||
| case int: | ||
| default: | ||
| assert.Failf(s.tester, "unexpected SafeContext type", "%T", v) | ||
| } | ||
| atomic.AddInt32(&s.backgroundHits, 1) | ||
| } | ||
| }) |
Comment on lines
+887
to
+906
| err := goPool.DefaultWorkerPool.Submit(func() { | ||
| for { | ||
| select { | ||
| case <-ev.stopBackground: | ||
| return | ||
| default: | ||
| } | ||
| switch v := c.SafeContext().(type) { | ||
| case nil: | ||
| case *safeCtxPayloadA: | ||
| _ = v.n | ||
| case *safeCtxPayloadB: | ||
| _ = v.s | ||
| case int: | ||
| default: | ||
| assert.Failf(ev.tester, "unexpected SafeContext type", "%T", v) | ||
| } | ||
| atomic.AddInt32(&ev.backgroundHits, 1) | ||
| } | ||
| }) |
Comment on lines
49
to
53
| - name: Setup and run golangci-lint | ||
| uses: golangci/golangci-lint-action@v7 | ||
| with: | ||
| version: v2.1.6 | ||
| version: v2.12.2 | ||
| args: -v -E gocritic -E misspell -E revive -E godot --timeout 5m |
Comment on lines
48
to
52
| - name: Setup and run golangci-lint | ||
| uses: golangci/golangci-lint-action@v7 | ||
| with: | ||
| version: v2.1.6 | ||
| version: v2.12.2 | ||
| args: -v -E gocritic -E misspell -E revive -E godot |
Comment on lines
48
to
52
| - name: Setup and run golangci-lint | ||
| uses: golangci/golangci-lint-action@v7 | ||
| with: | ||
| version: v2.1.6 | ||
| version: v2.12.2 | ||
| args: -v -E gocritic -E misspell -E revive -E godot |
Comment on lines
49
to
53
| - name: Setup and run golangci-lint | ||
| uses: golangci/golangci-lint-action@v7 | ||
| with: | ||
| version: v2.1.6 | ||
| version: v2.12.2 | ||
| args: -v -E gocritic -E misspell -E revive -E godot --timeout 5m |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #762 +/- ##
==========================================
+ Coverage 84.38% 84.80% +0.41%
==========================================
Files 22 22
Lines 2402 2422 +20
==========================================
+ Hits 2027 2054 +27
+ Misses 255 252 -3
+ Partials 120 116 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
No description provided.