diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000000000..b7d0e8966f7bd3 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,10 @@ +# Documentation for this file can be found at: +# https://docs.github.com/en/code-security/supply-chain-security/keeping-your-dependencies-updated-automatically/configuration-options-for-dependency-updates +version: 2 +updates: + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + commit-message: + prefix: ".github:" \ No newline at end of file diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 00000000000000..9c4efd7eac57b9 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,143 @@ +name: Build toolchain + +permissions: + contents: write + +on: + push: + branches: + - tailscale + - 'tailscale.go1.27' + pull_request: + branches: + - '*' + workflow_dispatch: + inputs: + ref: + description: Branch, commit or tag to build from + required: true + default: 'tailscale.go1.27' + skipTests: + description: Whether to skip tests. This should only be used in break-glass / extraordinary scenarios. + required: false + type: boolean + default: false + +jobs: + test: + runs-on: ubuntu-24.04 + if: ${{ !inputs.skipTests }} + steps: + - name: checkout + uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + with: + ref: ${{ inputs.ref || github.ref }} + - name: test + run: cd src && ./all.bash + + build_release: + strategy: + matrix: + GOOS: ["linux", "darwin", "windows"] + GOARCH: ["amd64", "arm64"] + runs-on: ubuntu-24.04 + if: contains(fromJSON('["push", "workflow_dispatch"]'), github.event_name) + steps: + - name: checkout + id: checkout + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ inputs.ref || github.ref }} + - name: set runtime/debug.tailscaleGitRev + run: sed -i "s/TAILSCALE_GIT_REV_TO_BE_REPLACED_AT_BUILD_TIME/${{ steps.checkout.outputs.commit }}/" src/runtime/debug/mod.go + - name: build + run: cd src && ./make.bash + env: + GOOS: "${{ matrix.GOOS }}" + GOARCH: "${{ matrix.GOARCH }}" + CGO_ENABLED: "" + - name: trim unnecessary bits + run: | + rm -rf pkg/*_* + mv pkg/tool/${{ matrix.GOOS }}_${{ matrix.GOARCH }} pkg + rm -rf pkg/tool/*_* + mv -f bin/${{ matrix.GOOS }}_${{ matrix.GOARCH }}/* bin/ || true + rm -rf bin/${{ matrix.GOOS }}_${{ matrix.GOARCH }} + mv pkg/${{ matrix.GOOS }}_${{ matrix.GOARCH }} pkg/tool + find . -type d -name 'testdata' -print0 | xargs -0 rm -rf + find . -name '*_test.go' -delete + - name: archive + run: cd .. && tar --exclude-vcs -zcf ${{ matrix.GOOS }}-${{ matrix.GOARCH }}.tar.gz go + - name: Set artifacts_path in env (workaround for actions/upload-artifact#176) + run: | + echo "artifacts_path=$(realpath ..)" >> $GITHUB_ENV + - name: save + uses: actions/upload-artifact@50769540e7f4bd5e21e526ee35c689e35e0d6874 # v4.4.0 + with: + name: ${{ matrix.GOOS }}-${{ matrix.GOARCH }} + path: ${{ env.artifacts_path }}/${{ matrix.GOOS }}-${{ matrix.GOARCH }}.tar.gz + + # GitHub releases are now immutable: once published, assets can no + # longer be added or changed. So we create the release as a draft, + # upload all assets to the draft, and only then publish it (in the + # publish_release job below). + create_release: + runs-on: ubuntu-24.04 + # By default, the "if" check here is skipped if one of the jobs in the "needs" block + # has been skipped. Adding `always()` ensures that we perform the check even if one + # of the jobs is skipped. We must check that the result of `build_release` is + # "success" and that `test` is one of "success" or "skipped" as a result to ensure + # we are only creating a release when the `test` job has suceeded or been skipped. + if: always() && contains(needs.build_release.result, 'success') && (contains(needs.test.result, 'success') || contains(needs.test.result, 'skipped')) && contains(fromJSON('["push", "workflow_dispatch"]'), github.event_name) + needs: [test, build_release] + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + REF: ${{ inputs.ref || github.sha }} + steps: + - name: create draft release + # Release name can't be the same as tag name, sigh + run: | + gh release create "build-$REF" \ + --title "$REF" \ + --target "$REF" \ + --draft \ + --prerelease + + upload_release: + strategy: + matrix: + GOOS: ["linux", "darwin", "windows"] + GOARCH: ["amd64", "arm64"] + runs-on: ubuntu-24.04 + # We need to do the `always()` hack here as well since the upstream `create_release` job + # needs it and this seems to transitively require that downstream jobs also have a similar + # check. + if: always() && contains(needs.create_release.result, 'success') && contains(fromJSON('["push", "workflow_dispatch"]'), github.event_name) + needs: [create_release] + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + REF: ${{ inputs.ref || github.sha }} + steps: + - name: download artifact + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + with: + name: ${{ matrix.GOOS }}-${{ matrix.GOARCH }} + - name: upload artifact to draft release + run: | + gh release upload "build-$REF" \ + "${{ matrix.GOOS }}-${{ matrix.GOARCH }}.tar.gz" + + publish_release: + runs-on: ubuntu-24.04 + if: always() && contains(needs.upload_release.result, 'success') && contains(fromJSON('["push", "workflow_dispatch"]'), github.event_name) + needs: [upload_release] + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + REF: ${{ inputs.ref || github.sha }} + steps: + - name: publish release + run: | + gh release edit "build-$REF" --draft=false diff --git a/Tailscale-Go.1.27.md b/Tailscale-Go.1.27.md new file mode 100644 index 00000000000000..671e8717ff95e4 --- /dev/null +++ b/Tailscale-Go.1.27.md @@ -0,0 +1,111 @@ +# Tailscale Go 1.27 fork preparation + +Status report for rebasing Tailscale's patches onto upstream Go 1.27. + +- **New branch:** `tailscale.go1.27`, based on upstream + `refs/heads/release-branch.go1.27` (at `go1.27rc2`, upstream commit + `b323620c2f91`). +- **Source branch:** `tailscale.go1.26` (21 carried patches vs. its + upstream merge-base, `go1.26.5`). +- **Result:** 13 patches cherry-picked; 8 dropped because they are + already upstream in Go 1.27. Every cherry-picked commit has + `[tailscale]` in its title and an `Updates tailscale/go#nnn` + reference. + +## Patches cherry-picked onto tailscale.go1.27 + +| Patch | Issue | Notes | +|---|---|---| +| runtime: add func TailscaleCurrentP | [#109](https://github.com/tailscale/go/issues/109) | clean pick | +| runtime: add TailscaleNumTimers for metrics | [#141](https://github.com/tailscale/go/issues/141) | clean pick; issue is missing the `tsgo` label (has only `need-upstream`, `ts26`) | +| cmd/link: add opt-in mechanism to fail if reflect is used | [#115](https://github.com/tailscale/go/issues/115) | clean pick | +| os: disable pidfd on Android | [#99](https://github.com/tailscale/go/issues/99) | clean pick; golang/go#70508 still unresolved for our supported Android versions | +| runtime/debug: embed Tailscale toolchain git rev | [#49](https://github.com/tailscale/go/issues/49) | clean pick | +| net: add TCP socket creation/close hooks to SockTrace API | [#58](https://github.com/tailscale/go/issues/58) | conflict: upstream `newFD` in `net/sock_posix.go` no longer returns an error; resolved | +| net, net/http: add enforcement hooks | [#55](https://github.com/tailscale/go/issues/55) | clean pick | +| net/http: add SetResponseWriteEnforcer | [#55](https://github.com/tailscale/go/issues/55) | **ported**: upstream deleted `h2_bundle.go` in favor of the new `net/http/internal/http2` package. The HTTP/2 hook now lives there as `http2.ResponseWriteEnforcer`, wired up via a new `net/http/tailscale_http2.go` (build-tagged `!nethttpomithttp2`) | +| .github: add Tailscale .github files | [#47](https://github.com/tailscale/go/issues/47) | branch references in `.github/workflows/build.yml` updated from `tailscale.go1.26` to `tailscale.go1.27` | +| cmd/go/internal/test: add opt-in file hashing instead of modtime for test caching (w/ git) | [#150](https://github.com/tailscale/go/issues/150) | clean pick | +| cmd/go: add -cachebinary build flag | [#149](https://github.com/tailscale/go/issues/149) | **replaced**: instead of re-picking the 1.26 branch's `go test -cachelink` commit, this picks patchset 4 of upstream [CL 739161](https://go-review.googlesource.com/c/go/+/739161), where the flag was renamed to `-cachebinary` and generalized to a build flag (default true for `go run`, false elsewhere) | +| cmd/go: set "tailscale_go" build tag | [#164](https://github.com/tailscale/go/issues/164) | clean pick | +| cmd/go: include tailscale.toolchain.rev in build cache salt | [#166](https://github.com/tailscale/go/issues/166) | clean pick | + +Housekeeping suggestion: add a `ts27` label to the 13 issues above, +matching the `ts24`/`ts26` convention. + +## Patches dropped (already upstream in Go 1.27) — issues to close + +| Patch | Upstream commit in 1.27 | Issue | Action | +|---|---|---|---| +| runtime: tolerate vendor suffixes in Linux kernel release strings | `6e808ba41a` | [#162](https://github.com/tailscale/go/issues/162) | **close** (open, already labeled `upstreamed`) | +| runtime: use uname version check for 64-bit time on 32-bit arch codepaths | `04dc12c1a1` | [#162](https://github.com/tailscale/go/issues/162) | same issue as above | +| runtime: fix value of ENOSYS on mips from 38 to 89 | `c918cbd556` | [#160](https://github.com/tailscale/go/issues/160) | **close** (open, labeled `upstreamed`) | +| internal/poll: move rsan to heap on windows | `e2ce40125f` | [#158](https://github.com/tailscale/go/issues/158) | **close** (open, labeled `upstreamed`) | +| net: don't wait 5 seconds to re-read /etc/resolv.conf | `6de7a19fea` (+ follow-up `2433a3f2d6`) | [#93](https://github.com/tailscale/go/issues/93) | **close** (open, labeled `upstreamed`) | +| cmd/go/internal/modfetch: quiet read-only filesystem stat cache warnings | `21c9de8c1d` | none | no issue was ever filed (see audit); upstreamed, nothing to do | +| syscall: define no-op Errno type on plan9 | `b5c2bd7e05` | none | no issue was ever filed (see audit); upstreamed, nothing to do | +| syscall: make plan9 Errno implement the error interface | `76ebf63307` | none | no issue was ever filed (see audit); upstreamed, nothing to do | + +Note: these patches were all absorbed into upstream by `go1.26.5` +already, so they contributed no diff between `tailscale.go1.26` and +its merge-base; dropping them for 1.27 loses nothing. + +## Audit: tailscale.go1.26 vs upstream release-branch.go1.26 + +Method: diffed `tailscale.go1.26` against its merge-base with upstream +`release-branch.go1.26` (`go1.26.5`, commit `c19862e5f8`) and checked +that every changed file is attributable to one of the 21 carried patch +commits. + +- **Result: clean.** All 36 changed files map to known patch commits. + No accidental/undocumented changes were smuggled in via merge + commits. +- **Findings — patches carried without a tailscale/go tracking issue** + (the process gap the audit was looking for): + 1. `cmd/go/internal/modfetch: quiet read-only filesystem stat cache warnings` + 2. `syscall: define no-op Errno type on plan9` + 3. `syscall: make plan9 Errno implement the error interface` + + All three were Tailscale-authored upstream CLs cherry-picked into + the fork while they waited for upstream review, and all three are in + Go 1.27, so no new issues are needed — but future interim + cherry-picks should get a tracking issue at merge time. +- Minor label housekeeping: [#141](https://github.com/tailscale/go/issues/141) + lacks the `tsgo` label. + +## Validation + +- `./make.bash` succeeds on `tailscale.go1.27` (linux/amd64, + bootstrapped with go1.26.3). +- **tsgotest** (`github.com/tailscale/tsgotest`): all tests pass + against the new toolchain (with the + `TAILSCALE_GIT_REV_TO_BE_REPLACED_AT_BUILD_TIME` placeholder + substituted, as CI does). +- In-tree `go test net/http -run 'Tailscale|Enforcer|ResponseWrite'` + passes, including the h1 and h2 subtests of + `TestSetResponseWriteEnforcer`, which exercises the ported HTTP/2 + hook. +- `go test -short runtime/debug syscall net net/http` passes. +- **tsgotest expanded** with two new tests: + - `TestIssue55_ResponseWriteEnforcerHTTP2`: the response write + enforcer on the HTTP/2 server path, which is a new distinct code + path after the 1.27 h2_bundle removal. + - `TestIssue58_NetSockTraceTCP`: the `DidCreateTCPConn` / + `WillCloseTCPConn` SockTrace hooks, whose wiring in + `net/sock_posix.go` was the site of a 1.27 merge conflict. + +## Follow-ups + +- [ ] Close issues [#162](https://github.com/tailscale/go/issues/162), + [#160](https://github.com/tailscale/go/issues/160), + [#158](https://github.com/tailscale/go/issues/158), + [#93](https://github.com/tailscale/go/issues/93) (all upstreamed + in Go 1.27). +- [ ] Add `ts27` labels to the 13 carried issues; add `tsgo` label to + [#141](https://github.com/tailscale/go/issues/141). +- [ ] Push `tailscale.go1.27` and the tsgotest additions after review. +- [x] 2026-07-30: merged upstream `release-branch.go1.27` (11 post-rc2 + fixes, through commit `95ae6418f3`); toolchain rebuilt and + tsgotest passes. +- [ ] Re-merge upstream `release-branch.go1.27` when go1.27.0 final is + tagged (branch is currently at go1.27rc2 plus post-rc2 fixes). diff --git a/api/go1.52109.txt b/api/go1.52109.txt new file mode 100644 index 00000000000000..e37b8276f20d7e --- /dev/null +++ b/api/go1.52109.txt @@ -0,0 +1 @@ +pkg runtime, func TailscaleCurrentP() int #109 diff --git a/api/go1.52141.txt b/api/go1.52141.txt new file mode 100644 index 00000000000000..c6e9ecb3cb8145 --- /dev/null +++ b/api/go1.52141.txt @@ -0,0 +1 @@ +pkg runtime, func TailscaleNumTimers() (int, int) #141 diff --git a/api/go1.5249.txt b/api/go1.5249.txt new file mode 100644 index 00000000000000..c2701388320e28 --- /dev/null +++ b/api/go1.5249.txt @@ -0,0 +1 @@ +pkg runtime/debug, func TailscaleToolchainGitRev() (string, bool) #49 diff --git a/api/go1.5255.txt b/api/go1.5255.txt new file mode 100644 index 00000000000000..1c1d6f35f9296c --- /dev/null +++ b/api/go1.5255.txt @@ -0,0 +1,4 @@ +pkg net, func SetDialEnforcer(func(context.Context, []Addr) error) #55 +pkg net, func SetResolveEnforcer(func(context.Context, string, string, string, Addr) error) #55 +pkg net/http, func SetRoundTripEnforcer(func(*Request) error) #55 +pkg net/http, func SetResponseWriteEnforcer(func(*Request)) #55 diff --git a/api/go1.5258.txt b/api/go1.5258.txt new file mode 100644 index 00000000000000..a8354ca1fa4291 --- /dev/null +++ b/api/go1.5258.txt @@ -0,0 +1,8 @@ +pkg net, func ContextSockTrace(context.Context) *SockTrace #58 +pkg net, func WithSockTrace(context.Context, *SockTrace) context.Context #58 +pkg net, type SockTrace struct #58 +pkg net, type SockTrace struct, DidCreateTCPConn func(syscall.RawConn) #58 +pkg net, type SockTrace struct, DidRead func(int) #58 +pkg net, type SockTrace struct, DidWrite func(int) #58 +pkg net, type SockTrace struct, WillCloseTCPConn func(syscall.RawConn) #58 +pkg net, type SockTrace struct, WillOverwrite func(*SockTrace) #58 diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 44cec1ae79e710..5a70b2b2977091 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -173,6 +173,18 @@ // Use -buildvcs=false to always omit version control information, or // -buildvcs=true to error out if version control information is available but // cannot be included due to a missing tool or ambiguous directory structure. +// -cachebinary +// Whether to store binaries produced by the linker in the build +// cache. This avoids relinking a binary when the same binary +// would be linked again, at the expense of a larger build cache. +// The default value depends on the subcommand: true for +// 'go run' and false otherwise. For example, 'go test' does not +// cache the (usually ephemeral) test binary by default, but +// caching it can avoid repeated relinking when running the same +// test binary repeatedly with different flags, environment, or +// other test inputs, such as when test wrappers shard tests +// within a package across multiple machines sharing a common +// build cache with GOCACHEPROG. // -compiler name // name of compiler to use, as in runtime.Compiler (gccgo or gc). // -gccgoflags '[pattern=]arg list' diff --git a/src/cmd/go/internal/cache/tailscale.go b/src/cmd/go/internal/cache/tailscale.go new file mode 100644 index 00000000000000..eda09f23e56019 --- /dev/null +++ b/src/cmd/go/internal/cache/tailscale.go @@ -0,0 +1,14 @@ +// Copyright 2026 Tailscale Inc. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cache + +import "runtime/debug" + +func init() { + if rev, ok := debug.TailscaleToolchainGitRev(); ok { + hashSalt = append(hashSalt, ' ') + hashSalt = append(hashSalt, rev...) + } +} diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go index 78cbbb949e7c27..a317f585d85eb5 100644 --- a/src/cmd/go/internal/cfg/cfg.go +++ b/src/cmd/go/internal/cfg/cfg.go @@ -71,6 +71,8 @@ var ( BuildA bool // -a flag BuildBuildmode string // -buildmode flag BuildBuildvcs = "auto" // -buildvcs flag: "true", "false", or "auto" + BuildCacheBinary bool // -cachebinary flag + BuildCacheBinarySet bool // whether -cachebinary was set explicitly BuildContext = defaultContext() BuildMod string // -mod flag BuildModExplicit bool // whether -mod was set explicitly @@ -113,6 +115,18 @@ var ( CGOChanged bool ) +// CacheBinary reports whether binaries produced by the linker should +// be stored in the build cache, using def as the default when the +// -cachebinary flag was not set explicitly. The default varies by +// subcommand: 'go run' caches binaries by default; other commands +// do not. +func CacheBinary(def bool) bool { + if BuildCacheBinarySet { + return BuildCacheBinary + } + return def +} + func defaultContext() build.Context { ctxt := build.Default diff --git a/src/cmd/go/internal/cfg/tailscale.go b/src/cmd/go/internal/cfg/tailscale.go new file mode 100644 index 00000000000000..859d3da9f8b2e2 --- /dev/null +++ b/src/cmd/go/internal/cfg/tailscale.go @@ -0,0 +1,9 @@ +// Copyright 2026 Tailscale Inc. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cfg + +func init() { + BuildContext.ToolTags = append(BuildContext.ToolTags, "tailscale_go") +} diff --git a/src/cmd/go/internal/githash/githash.go b/src/cmd/go/internal/githash/githash.go new file mode 100644 index 00000000000000..2682462f141b54 --- /dev/null +++ b/src/cmd/go/internal/githash/githash.go @@ -0,0 +1,182 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package githash + +import ( + "bytes" + "io/fs" + "os" + "os/exec" + "path" + "strconv" + "strings" + "sync" +) + +// GitHash is a git hash in hex form. +// +// It's usually a SHA-1 hash, but could be SHA-256 depending on the git +// configuration. +type GitHash string + +// Enabled is whether git hash lookups are enabled via the CMD_GO_USE_GIT_HASH +// environment variable. +var Enabled bool + +func init() { + s := os.Getenv("CMD_GO_USE_GIT_HASH") + if s != "" { + Enabled, _ = strconv.ParseBool(s) + } +} + +// gitHashKey is the key used to look up possible files in +// a git repo that match the same base name & size. +// +// This is used to avoid statting all files in a git repo +// when trying to find the git hash for a given file. +// Instead, we only stat files that match on name & size. +type gitHashKey struct { + baseName string // base name of file; as that's fs.FileInfo.Name gives us + size int64 +} + +type gitHashMap struct { + gitRoot string // absolute path to git repo root + + // cands is a list of files in the git repo, bucketed by their (base name, + // size) bucket key. This makes looking for a file faster later, without + // statting the whole world, yet still permitting lookup only from a + // fs.FileInfo that only has a base name & size & Sys info. + cands map[gitHashKey][]*gitHashCand +} + +type gitHashCand struct { + rel string // the relative git path from "git ls-files -r" + hash GitHash + + statOnce sync.Once + stat fs.FileInfo +} + +func (c *gitHashCand) getStat(m *gitHashMap) fs.FileInfo { + c.statOnce.Do(func() { + fullPath := path.Join(m.gitRoot, c.rel) + info, err := os.Lstat(fullPath) + if err == nil { + c.stat = info + } + }) + return c.stat +} + +var getGitHashMap = sync.OnceValue(buildGitHashMap) + +func buildGitHashMap() *gitHashMap { + m := &gitHashMap{ + cands: make(map[gitHashKey][]*gitHashCand), + } + gitRoot, err := exec.Command("git", "rev-parse", "--show-toplevel").Output() + if err != nil { + return nil + } + m.gitRoot = strings.TrimSpace(string(gitRoot)) + + cmd := exec.Command("git", "ls-tree", + "-r", // recursive + "--long", // include file sizes + "-z", // null-separated entries; don't have to deal with C quoting of some filenames + "HEAD", + ) + cmd.Dir = m.gitRoot // effectively git -C ; either way. + out, err := cmd.Output() + if err != nil { + return nil + } + // Parse lines of the form: + // + // 100644 blob cabbb1732c418125f9c773ce7a28ba34f2708554 639 .gitattributes + // 100644 blob 2b4a5fccdaf12f98cf8e255affa28cfd7e6a784d 95 .github/CODE_OF_CONDUCT.md + // + // .... but null-terminated instead of newline-terminated, so we don't have to deal + // with C quoting of filenames with certain characters. + // + // We don't care about the permissions. + remain := out + for len(remain) > 0 { + line, rest, ok := bytes.Cut(remain, []byte{0}) + if !ok { + break + } + remain = rest + meta, nameB, ok := bytes.Cut(line, []byte("\t")) + + _, hashAndSize, ok := bytes.Cut(meta, []byte(" blob ")) + if !ok { + continue + } + hashB, sizeB, ok := bytes.Cut(hashAndSize, []byte(" ")) + if !ok { + continue + } + size, err := strconv.ParseInt(strings.TrimSpace(string(sizeB)), 10, 64) + if err != nil { + continue + } + name := strings.TrimSpace(string(nameB)) + hash := strings.TrimSpace(string(hashB)) + k := gitHashKey{ + baseName: path.Base(name), + size: size, + } + m.cands[k] = append(m.cands[k], &gitHashCand{ + rel: name, + hash: GitHash(hash), + }) + } + return m +} + +// Hash returns the git hash for the given file info, if available. +func Hash(info fs.FileInfo) (GitHash, bool) { + if !Enabled || info == nil || !info.Mode().IsRegular() { + return "", false + } + k := gitHashKey{ + baseName: info.Name(), + size: info.Size(), + } + m := getGitHashMap() + if m == nil { + return "", false + } + for _, cand := range m.cands[k] { + if os.SameFile(info, cand.getStat(m)) { + return cand.hash, true + } + } + return "", false +} + +// ModTimeOrHash returns either the git hash (if enabled and available) or the +// mod time of the given file info. +// +// For non-regular files (notably directories), it returns nil if git hash is +// enabled. +// +// It always returns one of nil, time.Time, or GitHash (a string), all suitable +// for use in Sprintf verb %v. +func ModTimeOrHash(info fs.FileInfo) any { + if !Enabled { + return info.ModTime() + } + if h, ok := Hash(info); ok { + return h + } + if info.Mode().IsRegular() { + return info.ModTime() + } + return nil +} diff --git a/src/cmd/go/internal/modindex/read.go b/src/cmd/go/internal/modindex/read.go index 529c26ea057019..711d95aef4b573 100644 --- a/src/cmd/go/internal/modindex/read.go +++ b/src/cmd/go/internal/modindex/read.go @@ -28,6 +28,7 @@ import ( "cmd/go/internal/cache" "cmd/go/internal/cfg" "cmd/go/internal/fsys" + "cmd/go/internal/githash" "cmd/go/internal/imports" "cmd/go/internal/str" "cmd/internal/par" @@ -109,11 +110,11 @@ func dirHash(modroot, pkgdir string) (cache.ActionID, error) { if err != nil { return cache.ActionID{}, ErrNotIndexed } - if info.ModTime().After(cutoff) { + if !githash.Enabled && info.ModTime().After(cutoff) { return cache.ActionID{}, ErrNotIndexed } - fmt.Fprintf(h, "file %v %v %v\n", info.Name(), info.ModTime(), info.Size()) + fmt.Fprintf(h, "file %v %v %v\n", info.Name(), githash.ModTimeOrHash(info), info.Size()) } return h.Sum(), nil } diff --git a/src/cmd/go/internal/run/run.go b/src/cmd/go/internal/run/run.go index 1725c7d04f9090..d08a7d3f6f1d44 100644 --- a/src/cmd/go/internal/run/run.go +++ b/src/cmd/go/internal/run/run.go @@ -172,7 +172,7 @@ func runRun(ctx context.Context, cmd *base.Command, args []string) { } a1 := b.LinkAction(moduleLoader, work.ModeBuild, work.ModeBuild, p) - a1.CacheExecutable = true + a1.CacheExecutable = cfg.CacheBinary(true) // 'go run' caches binaries by default a := &work.Action{Mode: "go run", Actor: work.ActorFunc(buildRunProgram), Args: cmdArgs, Deps: []*work.Action{a1}} b.Do(ctx, a) } diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 2473c243e31159..c162416a9f434b 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -27,6 +27,7 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cache" "cmd/go/internal/cfg" + "cmd/go/internal/githash" "cmd/go/internal/load" "cmd/go/internal/lockedfile" "cmd/go/internal/modload" @@ -2113,7 +2114,7 @@ func hashOpen(name string) (cache.ActionID, error) { hashWriteStat(h, finfo) } } - } else if info.Mode().IsRegular() { + } else if info.Mode().IsRegular() && !githash.Enabled { // Because files might be very large, do not attempt // to hash the entirety of their content. Instead assume // the mtime and size recorded in hashWriteStat above @@ -2146,7 +2147,7 @@ func hashStat(name string) cache.ActionID { } func hashWriteStat(h io.Writer, info fs.FileInfo) { - fmt.Fprintf(h, "stat %d %x %v %v\n", info.Size(), uint64(info.Mode()), info.ModTime(), info.IsDir()) + fmt.Fprintf(h, "stat %d %x %v %v\n", info.Size(), uint64(info.Mode()), githash.ModTimeOrHash(info), info.IsDir()) } // testAndInputKey returns the actual cache key for the pair (testID, testInputsID). diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 8b3897e215ed57..c3721e049ea6ab 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -925,6 +925,11 @@ func (b *Builder) LinkAction(s *modload.Loader, mode, depMode BuildMode, p *load a := &Action{ Mode: "link", Package: p, + // Most commands do not store linked binaries in the build + // cache unless the -cachebinary flag asks for it. Commands + // that cache binaries by default ('go run', 'go tool', + // 'go doc') adjust CacheExecutable after LinkAction returns. + CacheExecutable: cfg.CacheBinary(false), } a1 := b.CompileAction(ModeBuild, depMode, p) diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go index 969e0029c3e294..90a4d7dc38753b 100644 --- a/src/cmd/go/internal/work/build.go +++ b/src/cmd/go/internal/work/build.go @@ -129,6 +129,18 @@ and test commands: Use -buildvcs=false to always omit version control information, or -buildvcs=true to error out if version control information is available but cannot be included due to a missing tool or ambiguous directory structure. + -cachebinary + Whether to store binaries produced by the linker in the build + cache. This avoids relinking a binary when the same binary + would be linked again, at the expense of a larger build cache. + The default value depends on the subcommand: true for + 'go run' and false otherwise. For example, 'go test' does not + cache the (usually ephemeral) test binary by default, but + caching it can avoid repeated relinking when running the same + test binary repeatedly with different flags, environment, or + other test inputs, such as when test wrappers shard tests + within a package across multiple machines sharing a common + build cache with GOCACHEPROG. -compiler name name of compiler to use, as in runtime.Compiler (gccgo or gc). -gccgoflags '[pattern=]arg list' @@ -323,6 +335,9 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) { cmd.Flag.Var(buildCompiler{}, "compiler", "") cmd.Flag.StringVar(&cfg.BuildBuildmode, "buildmode", "default", "") cmd.Flag.Var((*buildvcsFlag)(&cfg.BuildBuildvcs), "buildvcs", "") + if mask&OmitBuildOnlyFlags == 0 { + cmd.Flag.Var(cacheBinaryFlag{}, "cachebinary", "") + } cmd.Flag.Var(&load.BuildGcflags, "gcflags", "") cmd.Flag.Var(&load.BuildGccgoflags, "gccgoflags", "") if mask&OmitModFlag == 0 { @@ -419,6 +434,25 @@ func (f *buildvcsFlag) Set(s string) error { func (f *buildvcsFlag) String() string { return string(*f) } +// cacheBinaryFlag is the implementation of the -cachebinary flag. +// It records whether the flag was set explicitly, because its +// default value varies by subcommand. See cfg.CacheBinary. +type cacheBinaryFlag struct{} + +func (cacheBinaryFlag) IsBoolFlag() bool { return true } // allow -cachebinary (without arguments) + +func (cacheBinaryFlag) Set(s string) error { + b, err := strconv.ParseBool(s) + if err != nil { + return errors.New("value is not a valid bool") + } + cfg.BuildCacheBinary = b + cfg.BuildCacheBinarySet = true + return nil +} + +func (cacheBinaryFlag) String() string { return strconv.FormatBool(cfg.BuildCacheBinary) } + // fileExtSplit expects a filename and returns the name // and ext (without the dot). If the file has no // extension, ext will be empty. diff --git a/src/cmd/link/internal/ld/data.go b/src/cmd/link/internal/ld/data.go index 4a27db5a42ef88..f251240306932a 100644 --- a/src/cmd/link/internal/ld/data.go +++ b/src/cmd/link/internal/ld/data.go @@ -1246,6 +1246,17 @@ func addstrdata1(ctxt *Link, arg string) { pkg = objabi.PathToPrefix(pkg) name := pkg + arg[dot:eq] value := arg[eq+1:] + + // Handle "-X opt.forbidreflect=1" as another way to set -forbidreflect. + // This lets Tailscale set the -X way and work with either the upstream + // Go toolchain (that doesn't support link -forbidreflect) or the + // tailscale/go fork (that does). See https://github.com/tailscale/go/issues/115 + // for tracking getting this upstream. + if pkg == "opt" && name == "opt.forbidreflect" && value == "1" { + ctxt.ForbidReflect = true + return + } + if _, ok := strdata[name]; !ok { strnames = append(strnames, name) } diff --git a/src/cmd/link/internal/ld/deadcode.go b/src/cmd/link/internal/ld/deadcode.go index c540a8f21abc5f..691af3e783a587 100644 --- a/src/cmd/link/internal/ld/deadcode.go +++ b/src/cmd/link/internal/ld/deadcode.go @@ -476,6 +476,15 @@ func deadcode(ctxt *Link) { if *flagPruneWeakMap { d.mapinitcleanup() } + + if d.reflectSeen { + if ctxt.Debugvlog != 0 { + ctxt.Logf("deadcode: reflectSeen\n") + } + if ctxt.ForbidReflect { + panic("unexpected reflect with -forbidreflect") + } + } } // methodsig is a typed method signature (name + type). diff --git a/src/cmd/link/internal/ld/link.go b/src/cmd/link/internal/ld/link.go index c0d20680a709b9..1e36e6d6846fb1 100644 --- a/src/cmd/link/internal/ld/link.go +++ b/src/cmd/link/internal/ld/link.go @@ -87,6 +87,11 @@ type Link struct { Loaded bool // set after all inputs have been loaded as symbols + // ForbidReflect, if true, makes the deadcode pass panic if the binary ever + // has to enable the reflectSeen bit from seeing a SymFlagReflectMethod + // symbol. (method by name lookups, etc). + ForbidReflect bool + compressDWARF bool Libdir []string diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go index 19ed197a20b57e..81e46fe89523a3 100644 --- a/src/cmd/link/internal/ld/main.go +++ b/src/cmd/link/internal/ld/main.go @@ -204,6 +204,7 @@ func Main(arch *sys.Arch, theArch Arch) { } flagHeadType := flag.String("H", "", "set header `type`") flag.BoolVar(&ctxt.linkShared, "linkshared", false, "link against installed Go shared libraries") + flag.BoolVar(&ctxt.ForbidReflect, "forbidreflect", false, "error if reflect is used") flag.Var(&ctxt.LinkMode, "linkmode", "set link `mode`") flag.Var(&ctxt.BuildMode, "buildmode", "set build `mode`") flag.BoolVar(&ctxt.compressDWARF, "compressdwarf", true, "compress DWARF if possible") diff --git a/src/crypto/internal/fips140/mldsa/mldsa_test.go b/src/crypto/internal/fips140/mldsa/mldsa_test.go index 1c0239a30b516d..a2d2279a8e5841 100644 --- a/src/crypto/internal/fips140/mldsa/mldsa_test.go +++ b/src/crypto/internal/fips140/mldsa/mldsa_test.go @@ -346,7 +346,9 @@ func TestCASTRejectionPaths(t *testing.T) { testingOnlyRejectionReason = nil }) - fips140CAST() + if err := fips140CAST(); err != nil { + t.Fatal(err) + } for reason, hit := range reached { if !hit { diff --git a/src/crypto/internal/fips140/mlkem/cast.go b/src/crypto/internal/fips140/mlkem/cast.go index ea089c1b76c0c0..6799540a5daf7f 100644 --- a/src/crypto/internal/fips140/mlkem/cast.go +++ b/src/crypto/internal/fips140/mlkem/cast.go @@ -8,45 +8,75 @@ import ( "bytes" "crypto/internal/fips140" _ "crypto/internal/fips140/check" + "crypto/internal/fips140/sha256" "errors" "sync" ) var fipsSelfTest = sync.OnceFunc(func() { - fips140.CAST("ML-KEM-768", func() error { - var d = &[32]byte{ - 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, - 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, - 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, - 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, - } - var z = &[32]byte{ - 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, - 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, - 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, - 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, 0x40, - } - var m = &[32]byte{ - 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, - 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50, - 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, - 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x60, - } - var K = []byte{ - 0x55, 0x01, 0xfc, 0x52, 0x3b, 0x74, 0x5f, 0x41, - 0x76, 0x2a, 0x18, 0x8d, 0xe4, 0x4a, 0x59, 0xb9, - 0x20, 0xf4, 0x30, 0x14, 0x62, 0x04, 0xee, 0x4e, - 0x79, 0x37, 0x32, 0x39, 0x6d, 0xf7, 0xaa, 0x48, - } - dk := &DecapsulationKey768{} - kemKeyGen(dk, d, z) - ek := dk.EncapsulationKey() - var cc [CiphertextSize768]byte - Ke, _ := kemEncaps(&cc, ek, m) - Kd := kemDecaps(dk, &cc) - if !bytes.Equal(Ke, K) || !bytes.Equal(Kd, K) { - return errors.New("unexpected result") - } - return nil - }) + fips140.CAST("ML-KEM-768", fips140CAST) }) + +// fips140CAST covers both the successful and the implicit rejection paths, as +// required by IG 10.3.A, Resolution 14, and as tested by TestCASTRejectionPath. +// It tests only one parameter set as allowed by Note22. It compares dk and not +// ek, because ek is part of dk, as allowed by the same Resolution. It compares +// dk with an expected hash to avoid embedding several kilobytes of test vectors +// in every binary, as allowed by GeneralNote7. +func fips140CAST() error { + var d = &[32]byte{ + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, + 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, + 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f, 0x20, + } + var z = &[32]byte{ + 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28, + 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f, 0x30, + 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38, + 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, 0x40, + } + var dkHash = []byte{ + 0xa1, 0x32, 0x4a, 0x71, 0xf5, 0x7e, 0x26, 0x85, + 0x37, 0x7e, 0x4f, 0x7d, 0xe2, 0xd1, 0xf0, 0x58, + 0x53, 0x8e, 0x08, 0x7e, 0xe3, 0x9b, 0x48, 0x2b, + 0x3d, 0xbf, 0x96, 0xf9, 0xf7, 0x69, 0xc7, 0x85, + } + var m = &[32]byte{ + 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, 0x48, + 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, 0x50, + 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58, + 0x59, 0x5a, 0x5b, 0x5c, 0x5d, 0x5e, 0x5f, 0x60, + } + var K = []byte{ + 0x55, 0x01, 0xfc, 0x52, 0x3b, 0x74, 0x5f, 0x41, + 0x76, 0x2a, 0x18, 0x8d, 0xe4, 0x4a, 0x59, 0xb9, + 0x20, 0xf4, 0x30, 0x14, 0x62, 0x04, 0xee, 0x4e, + 0x79, 0x37, 0x32, 0x39, 0x6d, 0xf7, 0xaa, 0x48, + } + var Kr = []byte{ + 0x01, 0xa1, 0x03, 0x3a, 0x2e, 0x68, 0x33, 0x80, + 0x7f, 0xd3, 0x91, 0x80, 0xbe, 0x3b, 0x4f, 0x0a, + 0x1f, 0x6f, 0x50, 0x51, 0x67, 0x56, 0x9d, 0x0e, + 0x5e, 0x5e, 0x1f, 0xdb, 0x52, 0xa9, 0x10, 0xec, + } + dk := &DecapsulationKey768{} + kemKeyGen(dk, d, z) + ek := dk.EncapsulationKey() + H := sha256.New() + H.Write(TestingOnlyExpandedBytes768(dk)) + if !bytes.Equal(H.Sum(nil), dkHash) { + return errors.New("unexpected private key hash") + } + var cc [CiphertextSize768]byte + Kd := kemDecaps(dk, &cc) + if !bytes.Equal(Kd, Kr) { + return errors.New("unexpected rejection result") + } + Ke, _ := kemEncaps(&cc, ek, m) + Kd = kemDecaps(dk, &cc) + if !bytes.Equal(Ke, K) || !bytes.Equal(Kd, K) { + return errors.New("unexpected result") + } + return nil +} diff --git a/src/crypto/internal/fips140/mlkem/cast_test.go b/src/crypto/internal/fips140/mlkem/cast_test.go new file mode 100644 index 00000000000000..b28b6c01e860d6 --- /dev/null +++ b/src/crypto/internal/fips140/mlkem/cast_test.go @@ -0,0 +1,34 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package mlkem + +import "testing" + +func TestCASTRejectionPaths(t *testing.T) { + var rejected, notRejected bool + testingOnlyRejectionOutcome = func(compare int) { + if compare == 1 { + t.Log("non-rejection path hit") + notRejected = true + } else { + t.Log("rejection path hit") + rejected = true + } + } + t.Cleanup(func() { + testingOnlyRejectionOutcome = nil + }) + + if err := fips140CAST(); err != nil { + t.Fatal(err) + } + + if !rejected { + t.Error("rejection path not hit") + } + if !notRejected { + t.Error("non-rejection path not hit") + } +} diff --git a/src/crypto/internal/fips140/mlkem/mlkem768.go b/src/crypto/internal/fips140/mlkem/mlkem768.go index c4c3a9deaf4d7d..9c5e45e535ab1f 100644 --- a/src/crypto/internal/fips140/mlkem/mlkem768.go +++ b/src/crypto/internal/fips140/mlkem/mlkem768.go @@ -469,6 +469,8 @@ func (dk *DecapsulationKey768) Decapsulate(ciphertext []byte) (sharedKey []byte, return kemDecaps(dk, c), nil } +var testingOnlyRejectionOutcome func(int) + // kemDecaps produces a shared key from a ciphertext. // // It implements ML-KEM.Decaps_internal according to FIPS 203, Algorithm 18. @@ -488,6 +490,10 @@ func kemDecaps(dk *DecapsulationKey768, c *[CiphertextSize768]byte) (K []byte) { var cc [CiphertextSize768]byte c1 := pkeEncrypt(&cc, &dk.encryptionKey, (*[32]byte)(m), r) + if testingOnlyRejectionOutcome != nil { + testingOnlyRejectionOutcome(subtle.ConstantTimeCompare(c[:], c1)) + } + subtle.ConstantTimeCopy(subtle.ConstantTimeCompare(c[:], c1), Kout, Kprime) return Kout } diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go index c6d30841b6c2cf..d48316017806ae 100644 --- a/src/crypto/tls/conn.go +++ b/src/crypto/tls/conn.go @@ -99,12 +99,27 @@ type Conn struct { clientProtocol string // input/output - in, out halfConn - rawInput bytes.Buffer // raw input, starting with a record header - input bytes.Reader // application data waiting to be read, from rawInput.Next - hand bytes.Buffer // handshake data waiting to be read - buffering bool // whether records are buffered in sendBuf - sendBuf []byte // a buffer of records waiting to be sent + in, out halfConn + // rawInput holds raw input, starting with a record header. + // It is nil when no input is buffered, in which case the buffer has + // been returned to rawInputPool so that connections idle in Read do + // not pin a record-sized buffer. It is lazily repopulated from the + // pool by readFromUntil. + rawInput *bytes.Buffer + // smallInput is a small buffer that serves as rawInput while + // waiting for a record header after rawInput has been returned to + // rawInputPool. It is lazily allocated by readFromUntil and then + // kept for the life of the connection. + smallInput *bytes.Buffer + // input holds application data waiting to be read, from rawInput.Next. + input bytes.Reader + // hand holds handshake data waiting to be read. + // It is nil when no handshake data is buffered, in which case the + // buffer has been returned to handPool. Use handBuf and handLen to + // access it. + hand *bytes.Buffer + buffering bool // whether records are buffered in sendBuf + sendBuf []byte // a buffer of records waiting to be sent // bytesSent counts the bytes of application data sent. // packetsSent counts packets. @@ -580,7 +595,9 @@ func (e RecordHeaderError) Error() string { return "tls: " + e.Msg } func (c *Conn) newRecordHeaderError(conn net.Conn, msg string) (err RecordHeaderError) { err.Msg = msg err.Conn = conn - copy(err.RecordHeader[:], c.rawInput.Bytes()) + if c.rawInput != nil { + copy(err.RecordHeader[:], c.rawInput.Bytes()) + } return err } @@ -622,6 +639,19 @@ func (c *Conn) readRecordOrCCS(expectChangeCipherSpec bool) error { return c.in.setErrorLocked(errors.New("tls: internal error: attempted to read record with QUIC transport")) } + // If rawInput is empty, we are about to block in a Read on the + // underlying connection waiting for the next record, possibly for a + // long time. A previous record may have grown rawInput to the maximum + // record size; don't pin that memory while idle. Return the buffer to + // the pool, and let readFromUntil read the header into a small buffer + // and switch back to a pooled record-sized buffer only once the + // payload length is known. + if c.rawInput != nil && c.rawInput.Len() == 0 && c.rawInput != c.smallInput && c.rawInput.Cap() > maxIdleInputCap { + c.rawInput.Reset() + rawInputPool.Put(c.rawInput) + c.rawInput = nil + } + // Read header, payload. if err := c.readFromUntil(c.conn, recordHeaderLen); err != nil { // RFC 8446, Section 6.1 suggests that EOF without an alertCloseNotify @@ -702,7 +732,7 @@ func (c *Conn) readRecordOrCCS(expectChangeCipherSpec bool) error { } // Handshake messages MUST NOT be interleaved with other record types in TLS 1.3. - if c.vers == VersionTLS13 && typ != recordTypeHandshake && c.hand.Len() > 0 { + if c.vers == VersionTLS13 && typ != recordTypeHandshake && c.handLen() > 0 { return c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage)) } @@ -747,7 +777,7 @@ func (c *Conn) readRecordOrCCS(expectChangeCipherSpec bool) error { return c.in.setErrorLocked(c.sendAlert(alertDecodeError)) } // Handshake messages are not allowed to fragment across the CCS. - if c.hand.Len() > 0 { + if c.handLen() > 0 { return c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage)) } // In TLS 1.3, change_cipher_spec records are ignored until the @@ -783,7 +813,7 @@ func (c *Conn) readRecordOrCCS(expectChangeCipherSpec bool) error { if len(data) == 0 || expectChangeCipherSpec { return c.in.setErrorLocked(c.sendAlert(alertUnexpectedMessage)) } - c.hand.Write(data) + c.handBuf().Write(data) } return nil @@ -800,13 +830,85 @@ func (c *Conn) retryReadRecord(expectChangeCipherSpec bool) error { return c.readRecordOrCCS(expectChangeCipherSpec) } +// rawInputPool pools the record-sized buffers that back Conn.rawInput +// while records are being received. A connection returns its buffer to +// the pool before blocking to wait for a new record, often for a long +// time, so that idle connections do not each pin a record-sized buffer. +// Only buffers with capacity above maxIdleInputCap are pooled; smaller +// buffers stay attached to their connection. +var rawInputPool = sync.Pool{New: func() any { return new(bytes.Buffer) }} + +// maxIdleInputCap is the largest rawInput capacity that a connection +// keeps while waiting for a new record to arrive. It is large enough to +// hold a record header and small records, so that only connections +// receiving larger records pay for the pooled buffer switch below. +const maxIdleInputCap = 1024 + +// handPool pools the buffers that back Conn.hand, which typically grow +// to hold the peer's largest flight of handshake messages. A connection +// returns its buffer to the pool once the handshake completes and after +// buffered post-handshake messages have been consumed, so that +// established connections do not pin it. +var handPool = sync.Pool{New: func() any { return new(bytes.Buffer) }} + +// handBuf returns c.hand for writing, getting a buffer from handPool if +// c.hand is nil. +func (c *Conn) handBuf() *bytes.Buffer { + if c.hand == nil { + c.hand = handPool.Get().(*bytes.Buffer) + } + return c.hand +} + +// handLen returns the number of buffered handshake bytes. +func (c *Conn) handLen() int { + if c.hand == nil { + return 0 + } + return c.hand.Len() +} + +// releaseHand returns c.hand to handPool if it is empty. +func (c *Conn) releaseHand() { + if c.hand != nil && c.hand.Len() == 0 { + c.hand.Reset() + handPool.Put(c.hand) + c.hand = nil + } +} + // readFromUntil reads from r into c.rawInput until c.rawInput contains // at least n bytes or else returns an error. func (c *Conn) readFromUntil(r io.Reader, n int) error { + if c.rawInput == nil { + // The record buffer was released while waiting for a new + // record. Block for the header using the connection's small + // buffer; the switch to a pooled record-sized buffer below + // happens only once the payload length is known and data is + // flowing. + if c.smallInput == nil { + c.smallInput = new(bytes.Buffer) + } + c.rawInput = c.smallInput + } if c.rawInput.Len() >= n { return nil } needs := n - c.rawInput.Len() + if want := c.rawInput.Len() + needs + bytes.MinRead; want > maxIdleInputCap && want > c.rawInput.Cap() { + // Growing past maxIdleInputCap: switch to a pooled buffer so + // that record-sized buffers are recycled across connections + // rather than allocated for every record. + b := rawInputPool.Get().(*bytes.Buffer) + b.Write(c.rawInput.Bytes()) + if c.rawInput == c.smallInput { + c.smallInput.Reset() + } else if c.rawInput.Cap() > maxIdleInputCap { + c.rawInput.Reset() + rawInputPool.Put(c.rawInput) + } + c.rawInput = b + } // There might be extra input waiting on the wire. Make a best effort // attempt to fetch it so that it can be used in (*Conn).Read to // "predict" closeNotify alerts. @@ -1079,7 +1181,7 @@ func (c *Conn) readHandshakeBytes(n int) error { if c.quic != nil { return c.quicReadHandshakeBytes(n) } - for c.hand.Len() < n { + for c.handLen() < n { if err := c.readRecord(); err != nil { return err } @@ -1392,11 +1494,12 @@ func (c *Conn) Read(b []byte) (int, error) { if err := c.readRecord(); err != nil { return 0, err } - for c.hand.Len() > 0 { + for c.handLen() > 0 { if err := c.handlePostHandshakeMessage(); err != nil { return 0, err } } + c.releaseHand() } n, _ := c.input.Read(b) @@ -1574,6 +1677,14 @@ func (c *Conn) handshakeContext(ctx context.Context) (ret error) { panic("tls: internal error: handshake returned an error but is marked successful") } + // The handshake buffer typically grew to hold the peer's largest + // flight of handshake messages and is now empty. Post-handshake + // messages are rare and small, so release the buffer rather than + // pinning it for the life of the connection. + if c.handshakeErr == nil { + c.releaseHand() + } + if c.quic != nil { if c.handshakeErr == nil { c.quicHandshakeComplete() @@ -1685,7 +1796,7 @@ func (c *Conn) setReadTrafficSecret(suite *cipherSuiteTLS13, level QUICEncryptio // Ensure that there are no buffered handshake messages before changing the // read keys, since that can cause messages to be parsed that were encrypted // using old keys which are no longer appropriate. - if c.hand.Len() != 0 { + if c.handLen() != 0 { if locked { c.sendAlertLocked(alertUnexpectedMessage) } else { diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go index 2c96e2435ae156..b5a7ea6ae4f1e5 100644 --- a/src/crypto/tls/handshake_server_tls13.go +++ b/src/crypto/tls/handshake_server_tls13.go @@ -528,7 +528,7 @@ func (hs *serverHandshakeStateTLS13) doHelloRetryRequest(selectedGroup CurveID) // Make sure the client didn't send extra handshake messages alongside // their initial client_hello. If they sent two client_hello messages, // we will consume the second before they respond to the server_hello. - if c.hand.Len() != 0 { + if c.handLen() != 0 { c.sendAlert(alertUnexpectedMessage) return nil, errors.New("tls: handshake buffer not empty before HelloRetryRequest") } diff --git a/src/crypto/tls/quic.go b/src/crypto/tls/quic.go index b872d4fe0174a0..6755ebf58dedd3 100644 --- a/src/crypto/tls/quic.go +++ b/src/crypto/tls/quic.go @@ -288,9 +288,9 @@ func (q *QUICConn) HandleData(level QUICEncryptionLevel, data []byte) error { // The handshake goroutine has exited. c.handshakeMutex.Lock() defer c.handshakeMutex.Unlock() - c.hand.Write(c.quic.readbuf) + c.handBuf().Write(c.quic.readbuf) c.quic.readbuf = nil - for q.conn.hand.Len() >= 4 && q.conn.handshakeErr == nil { + for q.conn.handLen() >= 4 && q.conn.handshakeErr == nil { b := q.conn.hand.Bytes() n := int(b[1])<<16 | int(b[2])<<8 | int(b[3]) if n > maxHandshake { @@ -304,6 +304,7 @@ func (q *QUICConn) HandleData(level QUICEncryptionLevel, data []byte) error { q.conn.handshakeErr = err } } + q.conn.releaseHand() if q.conn.handshakeErr != nil { return quicError(q.conn.handshakeErr) } @@ -394,7 +395,7 @@ func quicError(err error) error { } func (c *Conn) quicReadHandshakeBytes(n int) error { - for c.hand.Len() < n { + for c.handLen() < n { if err := c.quicWaitForSignal(); err != nil { return err } @@ -407,7 +408,7 @@ func (c *Conn) quicSetReadSecret(level QUICEncryptionLevel, suite uint16, secret // read keys, since that can cause messages to be parsed that were encrypted // using old keys which are no longer appropriate. // TODO(roland): we should merge this check with the similar one in setReadTrafficSecret. - if c.hand.Len() != 0 { + if c.handLen() != 0 { c.sendAlert(alertUnexpectedMessage) return errors.New("tls: handshake buffer not empty before setting read traffic secret") } @@ -520,7 +521,7 @@ func (c *Conn) quicWaitForSignal() error { // The connection has been canceled. return c.sendAlertLocked(alertCloseNotify) } - c.hand.Write(c.quic.readbuf) + c.handBuf().Write(c.quic.readbuf) c.quic.readbuf = nil return nil } diff --git a/src/net/dial.go b/src/net/dial.go index af856256c080f4..bff91552b887e2 100644 --- a/src/net/dial.go +++ b/src/net/dial.go @@ -332,6 +332,24 @@ func parseNetwork(ctx context.Context, network string, needsProto bool) (afnet s return "", 0, UnknownNetworkError(network) } +// SetResolveEnforcer set a program-global resolver enforcer that can cause resolvers to +// fail based on the context and/or other arguments. +// +// f must be non-nil, it can only be called once, and must not be called +// concurrent with any dial/resolve. +func SetResolveEnforcer(f func(ctx context.Context, op, network, addr string, hint Addr) error) { + if f == nil { + panic("nil func") + } + if resolveEnforcer != nil { + panic("already called") + } + resolveEnforcer = f +} + +// resolveEnforcer, if non-nil, is the installed hook from SetResolveEnforcer. +var resolveEnforcer func(ctx context.Context, op, network, addr string, hint Addr) error + // resolveAddrList resolves addr using hint and returns a list of // addresses. The result contains at least one address when error is // nil. @@ -354,6 +372,13 @@ func (r *Resolver) resolveAddrList(ctx context.Context, op, network, addr string } return addrList{addr}, nil } + + if resolveEnforcer != nil { + if err := resolveEnforcer(ctx, op, network, addr, hint); err != nil { + return nil, err + } + } + addrs, err := r.internetAddrList(ctx, afnet, addr) if err != nil || op != "dial" || hint == nil { return addrs, err @@ -728,9 +753,32 @@ func (sd *sysDialer) dialParallel(ctx context.Context, primaries, fallbacks addr } } +// SetDialEnforcer set a program-global dial enforcer that can cause dials to +// fail based on the context and/or Addr(s). +// +// f must be non-nil, it can only be called once, and must not be called +// concurrent with any dial. +func SetDialEnforcer(f func(context.Context, []Addr) error) { + if f == nil { + panic("nil func") + } + if dialEnforcer != nil { + panic("already called") + } + dialEnforcer = f +} + +// dialEnforce, if non-nil, is any installed hook from SetDialEnforcer. +var dialEnforcer func(context.Context, []Addr) error + // dialSerial connects to a list of addresses in sequence, returning // either the first successful connection, or the first error. func (sd *sysDialer) dialSerial(ctx context.Context, ras addrList) (Conn, error) { + if dialEnforcer != nil { + if err := dialEnforcer(ctx, ras); err != nil { + return nil, err + } + } var firstErr error // The error from the first address is most relevant. for i, ra := range ras { diff --git a/src/net/fd_posix.go b/src/net/fd_posix.go index 023cd534e4206f..b97115e8984652 100644 --- a/src/net/fd_posix.go +++ b/src/net/fd_posix.go @@ -24,6 +24,12 @@ type netFD struct { net string laddr Addr raddr Addr + + // hooks (if provided) are called after successful reads or writes with the + // number of bytes transferred. + readHook func(int) + writeHook func(int) + closeHook func() } func (fd *netFD) name() string { @@ -47,6 +53,9 @@ func (fd *netFD) setAddr(laddr, raddr Addr) { func (fd *netFD) Close() error { // TODO Replace with runtime.AddCleanup. runtime.SetFinalizer(fd, nil) + if fd.closeHook != nil { + fd.closeHook() + } return fd.pfd.Close() } @@ -57,10 +66,16 @@ func (fd *netFD) shutdown(how int) error { } func (fd *netFD) closeRead() error { + if fd.closeHook != nil { + fd.closeHook() + } return fd.shutdown(syscall.SHUT_RD) } func (fd *netFD) closeWrite() error { + if fd.closeHook != nil { + fd.closeHook() + } return fd.shutdown(syscall.SHUT_WR) } @@ -89,18 +104,27 @@ func (fd *netFD) readFromInet6(p []byte, from *syscall.SockaddrInet6) (n int, er func (fd *netFD) readMsg(p []byte, oob []byte, flags int) (n, oobn, retflags int, sa syscall.Sockaddr, err error) { n, oobn, retflags, sa, err = fd.pfd.ReadMsg(p, oob, flags) + if fd.readHook != nil && err == nil { + fd.readHook(n) + } runtime.KeepAlive(fd) return n, oobn, retflags, sa, wrapSyscallError(readMsgSyscallName, err) } func (fd *netFD) readMsgInet4(p []byte, oob []byte, flags int, sa *syscall.SockaddrInet4) (n, oobn, retflags int, err error) { n, oobn, retflags, err = fd.pfd.ReadMsgInet4(p, oob, flags, sa) + if fd.readHook != nil && err == nil { + fd.readHook(n) + } runtime.KeepAlive(fd) return n, oobn, retflags, wrapSyscallError(readMsgSyscallName, err) } func (fd *netFD) readMsgInet6(p []byte, oob []byte, flags int, sa *syscall.SockaddrInet6) (n, oobn, retflags int, err error) { n, oobn, retflags, err = fd.pfd.ReadMsgInet6(p, oob, flags, sa) + if fd.readHook != nil && err == nil { + fd.readHook(n) + } runtime.KeepAlive(fd) return n, oobn, retflags, wrapSyscallError(readMsgSyscallName, err) } @@ -131,18 +155,27 @@ func (fd *netFD) writeToInet6(p []byte, sa *syscall.SockaddrInet6) (n int, err e func (fd *netFD) writeMsg(p []byte, oob []byte, sa syscall.Sockaddr) (n int, oobn int, err error) { n, oobn, err = fd.pfd.WriteMsg(p, oob, sa) + if fd.writeHook != nil && err == nil { + fd.writeHook(n) + } runtime.KeepAlive(fd) return n, oobn, wrapSyscallError(writeMsgSyscallName, err) } func (fd *netFD) writeMsgInet4(p []byte, oob []byte, sa *syscall.SockaddrInet4) (n int, oobn int, err error) { n, oobn, err = fd.pfd.WriteMsgInet4(p, oob, sa) + if fd.writeHook != nil && err == nil { + fd.writeHook(n) + } runtime.KeepAlive(fd) return n, oobn, wrapSyscallError(writeMsgSyscallName, err) } func (fd *netFD) writeMsgInet6(p []byte, oob []byte, sa *syscall.SockaddrInet6) (n int, oobn int, err error) { n, oobn, err = fd.pfd.WriteMsgInet6(p, oob, sa) + if fd.writeHook != nil && err == nil { + fd.writeHook(n) + } runtime.KeepAlive(fd) return n, oobn, wrapSyscallError(writeMsgSyscallName, err) } diff --git a/src/net/http/internal/http2/export_test.go b/src/net/http/internal/http2/export_test.go index 570fd3f8e5b20e..e9d7c6bead5d9d 100644 --- a/src/net/http/internal/http2/export_test.go +++ b/src/net/http/internal/http2/export_test.go @@ -25,6 +25,7 @@ func init() { const ( DefaultMaxReadFrameSize = defaultMaxReadFrameSize DefaultMaxStreams = defaultMaxStreams + HandlerChunkWriteSize = handlerChunkWriteSize InflowMinRefresh = inflowMinRefresh InitialHeaderTableSize = initialHeaderTableSize InitialMaxConcurrentStreams = initialMaxConcurrentStreams @@ -225,3 +226,15 @@ func InvalidHTTP1LookingFrameHeader() FrameHeader { func EncodeRequestHeaders(req *ClientRequest, addGzipHeader bool, peerMaxHeaderListSize uint64, headerf func(name, value string)) (httpcommon.EncodeHeadersResult, error) { return encodeRequestHeaders(req, addGzipHeader, peerMaxHeaderListSize, headerf) } + +func (w *responseWriter) hasWriteBuffer() bool { + return w.rws.bw != nil +} + +// ResponseWriterHasWriteBufferForTesting reports whether w (which must +// be or embed this package's responseWriter) currently holds a write +// buffer. It is for testing that Flush releases the buffer while a +// handler is parked mid-response. +func ResponseWriterHasWriteBufferForTesting(w any) bool { + return w.(interface{ hasWriteBuffer() bool }).hasWriteBuffer() +} diff --git a/src/net/http/internal/http2/frame.go b/src/net/http/internal/http2/frame.go index e3b371acec42a3..5567c2ae97a164 100644 --- a/src/net/http/internal/http2/frame.go +++ b/src/net/http/internal/http2/frame.go @@ -5,6 +5,7 @@ package http2 import ( + "bufio" "bytes" "encoding/binary" "errors" @@ -296,16 +297,29 @@ type Framer struct { maxReadSize uint32 headerBuf [frameHeaderLen]byte - // TODO: let getReadBuf be configurable, and use a less memory-pinning - // allocator in server.go to minimize memory pinned for many idle conns. - // Will probably also need to make frame invalidation have a hook too. - getReadBuf func(size uint32) []byte - readBuf []byte // cache for default getReadBuf + // readBuf is the buffer most recently returned by getReadBuf, + // reused for subsequent frames that fit in it. releaseReadBuf + // drops it before blocking to wait for a new frame header, so + // idle connections don't pin a frame-sized buffer. + readBuf []byte + // readBufP, if non-nil, is the pooled container holding readBuf's + // array, to be returned to readBufPool by releaseReadBuf. It is + // nil when readBuf is small (under maxIdleReadBufCap) and not + // worth pooling. + readBufP *[]byte maxWriteSize uint32 // zero means unlimited; TODO: implement - w io.Writer + w io.Writer + // wbuf is the frame currently being written, from startWrite's + // header through endWrite backfilling the length and writing it + // out. endWrite releases large buffers to writeBufPool so that + // idle connections don't pin a frame-sized buffer. wbuf []byte + // wbufP, if non-nil, is the pooled container holding wbuf's array, + // to be returned to writeBufPool by endWrite. It is nil when wbuf + // is small (under maxIdleWriteBufCap) and not worth pooling. + wbufP *[]byte // AllowIllegalWrites permits the Framer's Write methods to // write frames that do not conform to the HTTP/2 spec. This @@ -366,7 +380,27 @@ func (fr *Framer) maxHeaderValueCount() int { return fr.MaxHeaderValueCount } +// writeBufPool pools the frame-sized buffers that back Framer.wbuf. +// endWrite returns the buffer to the pool once the frame has been +// written out, so that idle connections don't each pin a buffer sized +// by the last frame they wrote. +var writeBufPool sync.Pool + +// maxIdleWriteBufCap is the largest wbuf capacity that a Framer keeps +// between frame writes. It is large enough for control frames and +// small HEADERS frames, so that only connections writing larger frames +// pay for the buffer pooling. +const maxIdleWriteBufCap = 1024 + func (f *Framer) startWrite(ftype FrameType, flags Flags, streamID uint32) { + if f.wbuf == nil { + // endWrite released the previous buffer; start with a pooled + // one if available. Appends below grow it as needed. + if bp, _ := writeBufPool.Get().(*[]byte); bp != nil { + f.wbufP = bp + f.wbuf = (*bp)[:0] + } + } // Write the FrameHeader. f.wbuf = append(f.wbuf[:0], 0, // 3 bytes of length, filled in endWrite @@ -399,9 +433,31 @@ func (f *Framer) endWrite() error { if err == nil && n != len(f.wbuf) { err = io.ErrShortWrite } + f.releaseWriteBuf() return err } +// releaseWriteBuf returns a large wbuf, whose frame has been written +// out, to writeBufPool, so that a connection idle between writes does +// not pin a frame-sized buffer. Small buffers stay attached to the +// Framer. +func (f *Framer) releaseWriteBuf() { + if cap(f.wbuf) <= maxIdleWriteBufCap { + return + } + bp := f.wbufP + if bp == nil { + bp = new([]byte) + } + // If appends outgrew a pooled array, the container is recycled + // with the new larger array and the old one is dropped, so the + // pool converges toward large buffers. + *bp = f.wbuf[:0] + writeBufPool.Put(bp) + f.wbufP = nil + f.wbuf = nil +} + func (f *Framer) logWrite() { if f.debugFramer == nil { f.debugFramerBuf = new(bytes.Buffer) @@ -464,15 +520,80 @@ func NewFramer(w io.Writer, r io.Reader) *Framer { debugReadLoggerf: log.Printf, debugWriteLoggerf: log.Printf, } - fr.getReadBuf = func(size uint32) []byte { - if cap(fr.readBuf) >= int(size) { - return fr.readBuf[:size] + fr.SetMaxReadFrameSize(maxFrameSize) + return fr +} + +// getReadBuf returns a buffer of the given size for ReadFrameForHeader +// to read a frame payload into. The returned buffer is only used until +// the next getReadBuf call, but the frame returned to the caller +// aliases it, so the buffer must not be reused until the frame is +// invalidated. The buffer is cached in readBuf, and large buffers are +// backed by readBufPool via readBufP. +func (fr *Framer) getReadBuf(size uint32) []byte { + if cap(fr.readBuf) >= int(size) { + return fr.readBuf[:size] + } + if int(size) > maxIdleReadBufCap { + // Reuse the pooled container we already hold, if any; + // otherwise get one from the pool. + bp := fr.readBufP + if bp == nil { + bp, _ = readBufPool.Get().(*[]byte) + if bp == nil { + bp = new([]byte) + } + } + if cap(*bp) < int(size) { + *bp = make([]byte, size) // drop the old array (don't Put it back), so the pool converges toward large buffers } - fr.readBuf = make([]byte, size) + fr.readBufP = bp + fr.readBuf = (*bp)[:size] return fr.readBuf } - fr.SetMaxReadFrameSize(maxFrameSize) - return fr + fr.readBufP = nil + fr.readBuf = make([]byte, size) + return fr.readBuf +} + +// readBufPool pools the frame-sized buffers that back Framer.readBuf. +// A Framer returns its buffer to the pool before blocking to wait for +// a new frame header, often for minutes or hours on idle connections, +// so that each idle connection does not pin a frame-sized buffer. +var readBufPool sync.Pool + +// maxIdleReadBufCap is the largest readBuf capacity that a Framer keeps +// while blocked waiting for a frame header to arrive. It is large +// enough for control frames and small HEADERS frames, so that only +// connections receiving larger frames pay for the buffer pooling. +const maxIdleReadBufCap = 1024 + +// releaseReadBuf invalidates the frame returned by the previous read +// and, if the read buffer came from readBufPool, returns it to the +// pool. It is called before blocking to wait for a new frame header. +func (fr *Framer) releaseReadBuf() { + if fr.readBufP == nil { + return + } + if br, ok := fr.r.(*bufio.Reader); ok && br.Buffered() > 0 { + // More frame data is already buffered: we are not about to + // block, and the buffer is about to be needed again. + return + } + fr.invalidateLastFrame() + *fr.readBufP = fr.readBuf[:0] + readBufPool.Put(fr.readBufP) + fr.readBufP = nil + fr.readBuf = nil +} + +// invalidateLastFrame invalidates the frame returned by the previous +// ReadFrameForHeader call, if any. +func (fr *Framer) invalidateLastFrame() { + if fr.lastFrame != nil { + fr.lastFrame.invalidate() + fr.lastFrame = nil + } } // SetMaxReadFrameSize sets the maximum size of a frame @@ -543,9 +664,7 @@ func (fr *Framer) ReadFrameHeader() (FrameHeader, error) { // It behaves identically to ReadFrame, other than not checking the maximum // frame size. func (fr *Framer) ReadFrameForHeader(fh FrameHeader) (Frame, error) { - if fr.lastFrame != nil { - fr.lastFrame.invalidate() - } + fr.invalidateLastFrame() payload := fr.getReadBuf(fh.Length) if _, err := io.ReadFull(fr.r, payload); err != nil { if fh == invalidHTTP1LookingFrameHeader() { @@ -581,6 +700,10 @@ func (fr *Framer) ReadFrameForHeader(fh FrameHeader) (Frame, error) { // If ReadFrame returns an error and a non-nil Frame, the Frame's StreamID // indicates the stream responsible for the error. func (fr *Framer) ReadFrame() (Frame, error) { + // The previous frame is invalid as of this call, so don't pin a + // frame-sized read buffer while waiting, possibly for a long time, + // for the next frame to arrive. + fr.releaseReadBuf() fh, err := fr.ReadFrameHeader() if err != nil { return nil, err diff --git a/src/net/http/internal/http2/frame_test.go b/src/net/http/internal/http2/frame_test.go index 43cd66bfc3bdc0..2d3318be9695c4 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -5,6 +5,7 @@ package http2 import ( + "bufio" "bytes" "fmt" "io" @@ -1605,3 +1606,162 @@ func TestTypeFrameParserHolePanic(t *testing.T) { t.Errorf("got %T; want *UnknownFrame", f) } } + +// frameAccessPanics reports whether accessing the given frame's payload +// panics, as it must after the frame has been invalidated by a +// subsequent ReadFrame call. +func frameAccessPanics(f func()) (panicked bool) { + defer func() { panicked = recover() != nil }() + f() + return +} + +func TestFramerReadBufPooling(t *testing.T) { + fr, _ := testFramer() + + // Reading a frame larger than maxIdleReadBufCap acquires a pooled buffer. + fr.WriteData(1, false, make([]byte, maxIdleReadBufCap+1)) + f1, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + if fr.readBufP == nil { + t.Error("after large frame: readBufP = nil, want pooled buffer") + } + if cap(fr.readBuf) <= maxIdleReadBufCap { + t.Errorf("after large frame: cap(readBuf) = %d, want > %d", cap(fr.readBuf), maxIdleReadBufCap) + } + + // The next ReadFrame releases the pooled buffer before blocking + // (bytes.Buffer is not a bufio.Reader, so there is no buffered + // look-ahead), invalidates the previous frame, and reads a payload + // of maxIdleReadBufCap or less into a small non-pooled buffer. + fr.WriteData(1, false, make([]byte, maxIdleReadBufCap)) + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + if fr.readBufP != nil { + t.Error("after small frame: readBufP != nil, want small non-pooled buffer") + } + if cap(fr.readBuf) > maxIdleReadBufCap { + t.Errorf("after small frame: cap(readBuf) = %d, want <= %d", cap(fr.readBuf), maxIdleReadBufCap) + } + if !frameAccessPanics(func() { f1.(*DataFrame).Data() }) { + t.Error("accessing previous frame's Data after ReadFrame did not panic") + } +} + +func TestFramerReadBufKeptWhileBuffered(t *testing.T) { + var wire bytes.Buffer + fw := NewFramer(&wire, nil) + fw.WriteData(1, false, make([]byte, maxIdleReadBufCap+1)) + fw.WriteData(1, false, make([]byte, maxIdleReadBufCap+1)) + + // With a bufio.Reader that already has the next frame buffered, + // ReadFrame is not about to block and keeps the pooled buffer. + fr := NewFramer(nil, bufio.NewReader(bytes.NewReader(wire.Bytes()))) + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + p1 := fr.readBufP + if p1 == nil { + t.Fatal("after large frame: readBufP = nil, want pooled buffer") + } + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + if fr.readBufP != p1 { + t.Error("pooled buffer was released despite buffered look-ahead data") + } +} + +func TestFramerReadBufGrowsInPlace(t *testing.T) { + var wire bytes.Buffer + fw := NewFramer(&wire, nil) + fw.WriteData(1, false, make([]byte, 2*maxIdleReadBufCap)) + fw.WriteData(1, false, make([]byte, 4*maxIdleReadBufCap)) + + // A larger frame arriving while the pooled buffer is retained + // (buffered look-ahead, so no release) grows the buffer within the + // same pooled container rather than acquiring a second one. + fr := NewFramer(nil, bufio.NewReader(bytes.NewReader(wire.Bytes()))) + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + p1 := fr.readBufP + if p1 == nil { + t.Fatal("after first frame: readBufP = nil, want pooled buffer") + } + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + if fr.readBufP != p1 { + t.Error("readBufP container not reused for larger frame") + } + if cap(fr.readBuf) < 4*maxIdleReadBufCap { + t.Errorf("cap(readBuf) = %d, want >= %d", cap(fr.readBuf), 4*maxIdleReadBufCap) + } +} + +func TestFramerReadBufReleasedOnReadError(t *testing.T) { + fr, _ := testFramer() + fr.WriteData(1, false, make([]byte, maxIdleReadBufCap+1)) + f1, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + + // A ReadFrame call that fails reading the next header still + // releases the buffer and invalidates the previous frame first. + if _, err := fr.ReadFrame(); err != io.EOF { + t.Fatalf("ReadFrame on empty input = %v, want io.EOF", err) + } + if fr.readBufP != nil { + t.Error("readBufP != nil after ReadFrame error, want released") + } + if !frameAccessPanics(func() { f1.(*DataFrame).Data() }) { + t.Error("accessing previous frame's Data after failed ReadFrame did not panic") + } +} + +func TestFramerWriteBufReleased(t *testing.T) { + fr, buf := testFramer() + if err := fr.WriteData(1, false, make([]byte, 2*maxIdleWriteBufCap)); err != nil { + t.Fatal(err) + } + if fr.wbuf != nil || fr.wbufP != nil { + t.Errorf("after large write: wbuf = %v (cap %d), wbufP = %v; want both released", + fr.wbuf != nil, cap(fr.wbuf), fr.wbufP != nil) + } + + // Subsequent writes still produce well-formed frames. + if err := fr.WritePing(false, [8]byte{1, 2, 3, 4, 5, 6, 7, 8}); err != nil { + t.Fatal(err) + } + if _, err := fr.ReadFrame(); err != nil { // the DATA frame + t.Fatal(err) + } + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + if _, ok := f.(*PingFrame); !ok { + t.Errorf("second frame = %T, want *PingFrame", f) + } + _ = buf +} + +func TestFramerWriteBufSmallKept(t *testing.T) { + fr, _ := testFramer() + fr.wbuf = make([]byte, 0, 64) // small buffer stays attached across writes + if err := fr.WritePing(false, [8]byte{}); err != nil { + t.Fatal(err) + } + if fr.wbuf == nil || cap(fr.wbuf) != 64 { + t.Errorf("after small write: cap(wbuf) = %d (nil=%v), want the seeded 64-byte buffer kept", + cap(fr.wbuf), fr.wbuf == nil) + } + if fr.wbufP != nil { + t.Error("after small write: wbufP != nil, want nil") + } +} diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index f4fbbc07a0be0d..bd2533a7c36e66 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -33,6 +33,7 @@ import ( "crypto/tls" "errors" "fmt" + "internal/synctest" "io" "log" "math" @@ -75,9 +76,20 @@ var ( var responseWriterStatePool = sync.Pool{ New: func() any { - rws := &responseWriterState{} - rws.bw = bufio.NewWriterSize(chunkWriter{rws}, handlerChunkWriteSize) - return rws + return &responseWriterState{} + }, +} + +// handlerWriterPool is a pool of the bufio.Writers used by +// responseWriterState (rws.bw) to buffer handler response writes. +// +// The buffers are acquired from the pool lazily on the first buffered +// write and, notably, are returned to it by Flush when empty, so that a +// handler that's parked mid-response for a long time (e.g. streaming a +// long poll) doesn't pin a buffer per stream. +var handlerWriterPool = sync.Pool{ + New: func() any { + return bufio.NewWriterSize(nil, handlerChunkWriteSize) }, } @@ -92,10 +104,6 @@ var ( type Server struct { mu sync.Mutex activeConns map[*serverConn]struct{} - - // Pool of error channels. This is per-Server rather than global - // because channels can't be reused across synctest bubbles. - errChanPool sync.Pool } func (s *Server) registerConn(sc *serverConn) { @@ -127,30 +135,33 @@ func (s *Server) startGracefulShutdown() { s.mu.Unlock() } -// Global error channel pool used for uninitialized Servers. -// We use a per-Server pool when possible to avoid using channels across synctest bubbles. +// errChanPool is a pool of reusable channels for reporting the result +// of a blocking frame write. +// +// The pool is not used inside synctest bubbles, since a channel created +// in one bubble can't be used from another bubble or from outside a +// bubble, and sync.Pool is not bubble-aware. var errChanPool = sync.Pool{ New: func() any { return make(chan error, 1) }, } -func (s *Server) getErrChan() chan error { - if s == nil { - return errChanPool.Get().(chan error) // Server used without calling ConfigureServer +func getErrChan() chan error { + if synctest.IsInBubble() { + // Channels can't be shared across synctest bubbles. + // Skip the pool; allocation cost is irrelevant in tests. + return make(chan error, 1) } - return s.errChanPool.Get().(chan error) + return errChanPool.Get().(chan error) } -func (s *Server) putErrChan(ch chan error) { - if s == nil { - errChanPool.Put(ch) // Server used without calling ConfigureServer - return +func putErrChan(ch chan error) { + if !synctest.IsInBubble() { + errChanPool.Put(ch) } - s.errChanPool.Put(ch) } func (s *Server) Configure(conf ServerConfig, tcfg *tls.Config) error { s.activeConns = make(map[*serverConn]struct{}) - s.errChanPool = sync.Pool{New: func() any { return make(chan error, 1) }} if tcfg.CipherSuites != nil && tcfg.MinVersion < tls.VersionTLS13 { // If they already provided a TLS 1.0–1.2 CipherSuite list, return an @@ -310,7 +321,19 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon if conf.CountError != nil { fr.countError = conf.CountError } - fr.ReadMetaHeaders = hpack.NewDecoder(uint32(conf.MaxDecoderHeaderTableSize), nil) + // A decoder table size below the initial 4096 (RFC 7540, Section 6.5.2) + // can't be applied immediately: the client may keep using the initial + // size until it processes our SETTINGS frame (RFC 7540, Section 6.5.3), + // and it signals the reduction with a dynamic table size update at the + // beginning of the first header block following the settings + // acknowledgment (RFC 7541, Section 4.2). Start at the initial size and + // lower it when the client acknowledges our SETTINGS. See processSettings. + decoderTableSize := uint32(conf.MaxDecoderHeaderTableSize) + if decoderTableSize < initialHeaderTableSize { + sc.pendingDecoderTableSize = decoderTableSize + decoderTableSize = initialHeaderTableSize + } + fr.ReadMetaHeaders = hpack.NewDecoder(decoderTableSize, nil) fr.MaxHeaderListSize = sc.maxHeaderListSize() fr.MaxHeaderValueCount = sc.hs.MaxHeaderValueCount() fr.SetMaxReadFrameSize(uint32(conf.MaxReadFrameSize)) @@ -432,6 +455,7 @@ type serverConn struct { sawFirstSettings bool // got the initial SETTINGS frame after the preface needToSendSettingsAck bool unackedSettings int // how many SETTINGS have we sent without ACKs? + pendingDecoderTableSize uint32 // if non-zero, HPACK decoder table size to apply on SETTINGS ack queuedControlFrames int // control frames in the writeSched queue clientMaxStreams uint32 // SETTINGS_MAX_CONCURRENT_STREAMS from client (our PUSH_PROMISE limit) advMaxStreams uint32 // our SETTINGS_MAX_CONCURRENT_STREAMS advertised the client @@ -998,7 +1022,7 @@ var writeDataPool = sync.Pool{ // writeDataFromHandler writes DATA response frames from a handler on // the given stream. func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte, endStream bool) error { - ch := sc.srv.getErrChan() + ch := getErrChan() writeArg := writeDataPool.Get().(*writeData) *writeArg = writeData{stream.id, data, endStream} err := sc.writeFrameFromHandler(FrameWriteRequest{ @@ -1030,7 +1054,7 @@ func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte, endStrea return errStreamClosed } } - sc.srv.putErrChan(ch) + putErrChan(ch) if frameWriteDone { writeDataPool.Put(writeArg) } @@ -1603,6 +1627,17 @@ func (sc *serverConn) processSettings(f *SettingsFrame) error { // hang up on them anyway. return sc.countError("ack_mystery", ConnectionError(ErrCodeProtocol)) } + if sc.pendingDecoderTableSize != 0 { + // The client has acknowledged our SETTINGS, so all header + // blocks it sends from now on were encoded with knowledge + // of our lower HEADER_TABLE_SIZE. It is now safe to apply + // the configured size to the decoder. The read goroutine + // is parked until readMore is called, so mutating the + // decoder here is race-free. + sc.framer.ReadMetaHeaders.SetAllowedMaxDynamicTableSize(sc.pendingDecoderTableSize) + sc.framer.ReadMetaHeaders.SetMaxDynamicTableSize(sc.pendingDecoderTableSize) + sc.pendingDecoderTableSize = 0 + } return nil } if f.NumSettings() > 100 || f.HasDuplicates() { @@ -2223,11 +2258,8 @@ func (sc *serverConn) newWriterAndRequestNoBody(st *stream, rp httpcommon.Server func (sc *serverConn) newResponseWriter(st *stream) *responseWriter { rws := responseWriterStatePool.Get().(*responseWriterState) - bwSave := rws.bw *rws = responseWriterState{} // zero all the fields rws.conn = sc - rws.bw = bwSave - rws.bw.Reset(chunkWriter{rws}) rws.stream = st return &responseWriter{rws: rws} } @@ -2335,7 +2367,7 @@ func (sc *serverConn) writeHeaders(st *stream, headerData *writeResHeaders) erro // waiting for this frame to be written, so an http.Flush mid-handler // writes out the correct value of keys, before a handler later potentially // mutates it. - errc = sc.srv.getErrChan() + errc = getErrChan() } if err := sc.writeFrameFromHandler(FrameWriteRequest{ write: headerData, @@ -2347,7 +2379,7 @@ func (sc *serverConn) writeHeaders(st *stream, headerData *writeResHeaders) erro if errc != nil { select { case err := <-errc: - sc.srv.putErrChan(errc) + putErrChan(errc) return err case <-sc.doneServing: return errClientDisconnected @@ -2478,7 +2510,13 @@ type responseWriterState struct { conn *serverConn // TODO: adjust buffer writing sizes based on server config, frame size updates from peer, etc - bw *bufio.Writer // writing to a chunkWriter{this *responseWriterState} + // + // bw buffers handler writes, writing to a chunkWriter{this + // *responseWriterState}. It is nil until the first buffered write + // (see responseWriter.write) and is returned to handlerWriterPool + // (and set nil again) whenever a Flush leaves it empty, so a + // handler parked mid-response doesn't pin a buffer. + bw *bufio.Writer // mutated by http.Handler goroutine: handlerHeader Header // nil until called @@ -2760,12 +2798,19 @@ func (w *responseWriter) Flush() { func (w *responseWriter) FlushError() error { rws := w.rws if rws == nil { - panic("Header called after Handler finished") + panic("Flush called after Handler finished") } var err error - if rws.bw.Buffered() > 0 { + if rws.bw != nil && rws.bw.Buffered() > 0 { err = rws.bw.Flush() + if err == nil { + rws.releaseWriteBuffer() + } } else { + if rws.bw != nil { + // If a >4KB write allocated a bufio before it flushed itself, release. + rws.releaseWriteBuffer() + } // The bufio.Writer won't call chunkWriter.Write // (writeChunk with zero bytes), so we have to do it // ourselves to force the HTTP response header and/or @@ -2838,10 +2883,18 @@ func (w *responseWriter) WriteHeader(code int) { rws.writeHeader(code) } +// ResponseWriteEnforcer, if non-nil, is called with the request before the +// server writes a response header. +// It is set at most once, by net/http.SetResponseWriteEnforcer, during init. +var ResponseWriteEnforcer func(*ServerRequest) + func (rws *responseWriterState) writeHeader(code int) { if rws.wroteHeader { return } + if ResponseWriteEnforcer != nil { + ResponseWriteEnforcer(&rws.req) + } checkWriteHeaderCode(code) @@ -2919,6 +2972,10 @@ func (w *responseWriter) write(lenData int, dataB []byte, dataS string) (n int, return 0, errors.New("http2: handler wrote more than declared Content-Length") } + if rws.bw == nil { + rws.bw = handlerWriterPool.Get().(*bufio.Writer) + rws.bw.Reset(chunkWriter{rws}) + } if dataB != nil { return rws.bw.Write(dataB) } else { @@ -2926,10 +2983,23 @@ func (w *responseWriter) write(lenData int, dataB []byte, dataS string) (n int, } } +// releaseWriteBuffer returns rws.bw to handlerWriterPool. +func (rws *responseWriterState) releaseWriteBuffer() { + bw := rws.bw + rws.bw = nil + bw.Reset(nil) // don't retain the chunkWriter's rws pointer in the pool + handlerWriterPool.Put(bw) +} + func (w *responseWriter) handlerDone() { rws := w.rws rws.handlerDone = true w.Flush() + if rws.bw != nil { + // A failed Flush left data (and a sticky error) behind; + // discard both and recycle the buffer. + rws.releaseWriteBuffer() + } w.rws = nil responseWriterStatePool.Put(rws) } @@ -3015,7 +3085,7 @@ func (w *responseWriter) Push(target, method string, header Header) error { method: method, url: u, header: cloneHeader(header), - done: sc.srv.getErrChan(), + done: getErrChan(), } select { @@ -3032,7 +3102,7 @@ func (w *responseWriter) Push(target, method string, header Header) error { case <-st.cw: return errStreamClosed case err := <-msg.done: - sc.srv.putErrChan(msg.done) + putErrChan(msg.done) return err } } diff --git a/src/net/http/internal/http2/server_test.go b/src/net/http/internal/http2/server_test.go index 3d42cd4a4ac404..f15ad9e666cb10 100644 --- a/src/net/http/internal/http2/server_test.go +++ b/src/net/http/internal/http2/server_test.go @@ -2368,6 +2368,80 @@ func testServer_Response_Empty_Data_Not_FlowControlled(t *testing.T) { }) } +// TestServer_Response_FlushReleasesWriteBuffer verifies that a handler's +// write buffer is released back to the pool by an empty-leaving Flush and +// lazily reacquired by the next write, so that handlers parked mid-response +// (long polls) don't pin a 4KB buffer per stream. +func TestServer_Response_FlushReleasesWriteBuffer(t *testing.T) { + synctest.Test(t, testServer_Response_FlushReleasesWriteBuffer) +} +func testServer_Response_FlushReleasesWriteBuffer(t *testing.T) { + const msg = "hello, " + const msg2 = "world" + largeMsg := bytes.Repeat([]byte("a"), HandlerChunkWriteSize*2) + testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error { + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer allocated before first write") + } + io.WriteString(w, msg) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not allocated after buffered write") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by Flush") + } + io.WriteString(w, msg2) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not reacquired by write after Flush") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by second Flush") + } + // A []byte write larger than the 4KB write buffer bypasses + // the buffer entirely, going directly to the chunkWriter and + // leaving the buffer allocated but empty. Flush must + // release it in that case too. + w.Write(largeMsg) + if !ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not allocated by large write") + } + w.(http.Flusher).Flush() + if ResponseWriterHasWriteBufferForTesting(w) { + return fmt.Errorf("write buffer not released by Flush after buffer-bypassing write") + } + return nil + }, func(st *serverTester) { + getSlash(st) + st.wantHeaders(wantHeader{ + streamID: 1, + endStream: false, + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: []byte(msg), + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: []byte(msg2), + }) + st.wantData(wantData{ + streamID: 1, + endStream: false, + data: largeMsg, + multiple: true, + }) + st.wantData(wantData{ + streamID: 1, + endStream: true, + size: 0, + }) + }) +} + func TestServer_Response_Automatic100Continue(t *testing.T) { synctest.Test(t, testServer_Response_Automatic100Continue) } @@ -2750,6 +2824,88 @@ func testServer_MaxDecoderHeaderTableSize(t *testing.T) { } } +func TestServer_MaxDecoderHeaderTableSize_DeferredUntilAck(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + testServer_MaxDecoderHeaderTableSizeDeferred(t, true) + }) +} + +func TestServer_MaxDecoderHeaderTableSize_EnforcedAfterAck(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + testServer_MaxDecoderHeaderTableSizeDeferred(t, false) + }) +} + +// testServer_MaxDecoderHeaderTableSizeDeferred tests that a server +// configured with a decoder header table size below the protocol-initial +// 4096 bytes does not apply it until the client acknowledges the server's +// SETTINGS frame. Until then, the client's encoder may legitimately +// reference dynamic table entries under the initial 4096-byte table size +// (RFC 7540, Section 6.5.3; RFC 7541, Section 4.2). +func testServer_MaxDecoderHeaderTableSizeDeferred(t *testing.T, sizeUpdateAfterAck bool) { + st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {}, func(h2 *http.HTTP2Config) { + h2.MaxDecoderHeaderTableSize = 1 + }) + defer st.Close() + + st.writePreface() + st.writeSettings() + st.sync() + readFrame[*SettingsFrame](t, st) + + // The server's initial WINDOW_UPDATE and its ack of our SETTINGS + // can arrive in either order. + st.wantUnorderedFrames( + func(f *SettingsFrame) bool { + if !f.IsAck() { + t.Fatalf("got second non-ack SETTINGS frame") + } + return true + }, + func(f *WindowUpdateFrame) bool { + return true + }, + ) + + // Send two requests whose header blocks use the dynamic table, before + // acknowledging the server's SETTINGS. The second request references + // entries added by the first. Our encoder is still permitted to use + // the initial 4096-byte table size, so both must succeed. + for _, id := range []uint32{1, 3} { + st.writeHeaders(HeadersFrameParam{ + StreamID: id, + BlockFragment: st.encodeHeader("x-custom-header", "long-enough-value-to-index"), + EndStream: true, + EndHeaders: true, + }) + st.wantHeaders(wantHeader{streamID: id, endStream: true}) + } + + // Acknowledge the server's SETTINGS. Header blocks we send from here + // on must respect the reduced table size. + st.writeSettingsAck() + st.sync() + + if sizeUpdateAfterAck { + // A compliant encoder begins its next header block with a + // dynamic table size update. The request must succeed. + st.hpackEnc.SetMaxDynamicTableSize(1) + } + st.writeHeaders(HeadersFrameParam{ + StreamID: 5, + BlockFragment: st.encodeHeader("x-custom-header", "long-enough-value-to-index"), + EndStream: true, + EndHeaders: true, + }) + if sizeUpdateAfterAck { + st.wantHeaders(wantHeader{streamID: 5, endStream: true}) + } else { + // A header block that keeps referencing the now-evicted + // 4096-byte table without a size update must be rejected. + st.wantGoAway(5, ErrCodeCompression) + } +} + func TestServer_MaxEncoderHeaderTableSize(t *testing.T) { synctest.Test(t, testServer_MaxEncoderHeaderTableSize) } diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go index 0b32ea72da0246..9d63ed26c85cfc 100644 --- a/src/net/http/internal/http2/transport.go +++ b/src/net/http/internal/http2/transport.go @@ -247,6 +247,27 @@ type clientStream struct { donec chan struct{} // closed after the stream is in the closed state on100 chan struct{} // buffered; written to if a 100 is received + // detached, guarded by cc.mu, indicates that the writeRequest + // goroutine has exited without waiting for the stream to end, and + // that cleanupWriteRequest should instead be run (on a new goroutine) + // by whichever of abortStreamLocked or clientConnReadLoop.endStream + // ends the stream. It is cleared when that cleanup is scheduled. + // See clientStream.detach. + detached bool + + // stopCtxWatch, if non-nil, cancels the context.AfterFunc watching + // for request context cancellation on behalf of a detached stream. + // It is set (under cc.mu) at most once, by detach, before detached + // is set, and is called by cleanupWriteRequest. + stopCtxWatch func() bool + + // respHeaderTimeoutTimer, guarded by cc.mu, is a timer enforcing + // Transport.ResponseHeaderTimeout on behalf of a detached stream. + // It is armed by detach if response headers haven't yet arrived, and + // stopped when they do (clientConnReadLoop.processHeaders) or when + // the stream ends (cleanupWriteRequest). + respHeaderTimeoutTimer *time.Timer + respHeaderRecv chan struct{} // closed when headers are received res *ClientResponse // set if respHeaderRecv is closed @@ -299,6 +320,10 @@ func (cs *clientStream) abortStreamLocked(err error) { cs.abortErr = err close(cs.abort) }) + if cs.detached { + cs.detached = false + go cs.cleanupWriteRequest(cs.abortErr) + } if cs.reqBody != nil { cs.closeReqBodyLocked() } @@ -1211,12 +1236,83 @@ func (cc *ClientConn) roundTrip(req *ClientRequest, streamf func(*clientStream)) // doRequest runs for the duration of the request lifetime. // -// It sends the request and performs post-request cleanup (closing Request.Body, etc.). +// It sends the request and performs post-request cleanup (closing Request.Body, etc.), +// except when writeRequest detaches from the stream, in which case cleanup is +// performed at stream end by whoever ends it. See clientStream.detach. func (cs *clientStream) doRequest(req *ClientRequest, streamf func(*clientStream)) { err := cs.writeRequest(req, streamf) + if err == errStreamDetached { + return + } cs.cleanupWriteRequest(err) } +// errStreamDetached is a sentinel returned by writeRequest to tell doRequest +// that the stream detached and cleanupWriteRequest will be called at stream +// end by whoever ends it. It is never returned to users. +var errStreamDetached = errors.New("http2: internal sentinel; stream detached from writeRequest goroutine") + +// detach arranges for cleanupWriteRequest to run when the stream ends (the +// peer half-closes it, it's aborted, or the request context is canceled), +// letting the writeRequest goroutine exit instead of parking until then. +// +// This matters for servers and proxies with many concurrent long-lived +// response streams (long polls): without it, each in-flight request pins a +// goroutine and its stack for the stream's lifetime doing nothing but +// waiting. +// +// respHeaderTimeout, if non-zero, gives the Transport.ResponseHeaderTimeout +// to enforce on the detached stream if response headers haven't arrived yet. +// +// It reports whether the stream was detached. It returns false if the stream +// has already ended, in which case the caller should wait for the stream end +// events itself (they're already pending). +func (cs *clientStream) detach(respHeaderTimeout time.Duration) bool { + cc := cs.cc + cc.mu.Lock() + defer cc.mu.Unlock() + select { + case <-cs.peerClosed: + return false + case <-cs.abort: + return false + default: + } + if respHeaderTimeout != 0 { + select { + case <-cs.respHeaderRecv: + // Headers already arrived; nothing to enforce. + default: + cs.respHeaderTimeoutTimer = time.AfterFunc(respHeaderTimeout, func() { + cc.mu.Lock() + defer cc.mu.Unlock() + select { + case <-cs.respHeaderRecv: + // Headers arrived after all; we lost a race + // with the Stop in processHeaders. Not a + // timeout. + return + default: + } + cs.abortStreamLocked(errTimeout) + }) + } + } + // Watch for request context cancellation without parking a goroutine + // on ctx.Done(). If the context was canceled already, AfterFunc runs + // the func in a new goroutine, which blocks acquiring cc.mu until we + // return. + // + // stopCtxWatch must be assigned before detached is set: once detached + // is set, an abort or peer close can schedule cleanupWriteRequest + // (which calls stopCtxWatch) as soon as we release cc.mu. + cs.stopCtxWatch = context.AfterFunc(cs.ctx, func() { + cs.abortStream(cs.ctx.Err()) + }) + cs.detached = true + return true +} + var errExtendedConnectNotSupported = errors.New("net/http: extended connect not supported by peer") // writeRequest sends a request. @@ -1340,6 +1436,24 @@ func (cs *clientStream) writeRequest(req *ClientRequest, streamf func(*clientStr traceWroteRequest(cs.trace, err) + // If the request is fully sent and there's nothing left for this + // goroutine to do but wait for the stream to end, detach from the + // stream and exit rather than pinning this goroutine (and its stack) + // for the lifetime of what may be a very long-lived response stream. + // The remaining cases below then run cleanupWriteRequest from the + // stream-end event sites instead: + // - peerClosed and abort schedule it directly + // (abortStreamLocked, clientConnReadLoop.endStream) + // - ctx.Done is handled via context.AfterFunc in detach + // - ResponseHeaderTimeout is enforced by a time.AfterFunc timer, + // armed in detach and stopped when headers arrive + // The deprecated Request.Cancel channel can only be watched by a + // goroutine, so that (rare) case keeps the historical behavior of + // waiting here. + if cs.sentEndStream && cs.reqCancel == nil && cs.detach(cc.responseHeaderTimeout()) { + return errStreamDetached + } + var respHeaderTimer <-chan time.Time var respHeaderRecv chan struct{} if d := cc.responseHeaderTimeout(); d != 0 { @@ -1348,6 +1462,7 @@ func (cs *clientStream) writeRequest(req *ClientRequest, streamf func(*clientStr respHeaderTimer = timer.C respHeaderRecv = cs.respHeaderRecv } + // Wait until the peer half-closes its end of the stream, // or until the request is aborted (via context, error, or otherwise), // whichever comes first. @@ -1433,6 +1548,10 @@ func encodeRequestHeaders(req *ClientRequest, addGzipHeader bool, peerMaxHeaderL func (cs *clientStream) cleanupWriteRequest(err error) { cc := cs.cc + if cs.stopCtxWatch != nil { + cs.stopCtxWatch() + } + if cs.ID == 0 { // We were canceled before creating the stream, so return our reservation. cc.decrStreamReservations() @@ -1443,6 +1562,10 @@ func (cs *clientStream) cleanupWriteRequest(err error) { // and in multiple cases: server replies <=299 and >299 // while still writing request body cc.mu.Lock() + if t := cs.respHeaderTimeoutTimer; t != nil { + t.Stop() + cs.respHeaderTimeoutTimer = nil + } mustCloseBody := false if cs.reqBody != nil && cs.reqBodyClosed == nil { mustCloseBody = true @@ -2154,6 +2277,13 @@ func (rl *clientConnReadLoop) processHeaders(f *MetaHeadersFrame) error { } cs.res = res close(cs.respHeaderRecv) + // Stop a detached stream's response header timeout, if armed. + rl.cc.mu.Lock() + if t := cs.respHeaderTimeoutTimer; t != nil { + t.Stop() + cs.respHeaderTimeoutTimer = nil + } + rl.cc.mu.Unlock() if f.StreamEnded() { rl.endStream(cs) } @@ -2566,6 +2696,10 @@ func (rl *clientConnReadLoop) endStream(cs *clientStream) { defer rl.cc.mu.Unlock() cs.bufPipe.closeWithErrorAndCode(io.EOF, cs.copyTrailers) close(cs.peerClosed) + if cs.detached { + cs.detached = false + go cs.cleanupWriteRequest(nil) + } } } diff --git a/src/net/http/internal/http2/transport_test.go b/src/net/http/internal/http2/transport_test.go index e3093f64e39380..6cbd8cb676a431 100644 --- a/src/net/http/internal/http2/transport_test.go +++ b/src/net/http/internal/http2/transport_test.go @@ -26,6 +26,7 @@ import ( "net/url" "os" "reflect" + "runtime" "sort" "strconv" "strings" @@ -5651,3 +5652,128 @@ func testExtendedConnectReadFrameError(t *testing.T) { t.Fatalf("after connection closed: RoundTrip succeeded; want error") } } + +// TestTransportRequestGoroutineExits verifies that the goroutine spawned to +// write a request exits once the request has been fully sent, rather than +// parking for the lifetime of the response stream. For clients with many +// concurrent long-lived streams (long polls), a parked goroutine and its +// stack per stream is a significant memory cost. +func TestTransportRequestGoroutineExits(t *testing.T) { + synctest.Test(t, testTransportRequestGoroutineExits) +} +func testTransportRequestGoroutineExits(t *testing.T) { + tc := newTestClientConn(t) + tc.greet() + + // Count the goroutines net/http has parked (the connection's read + // loop, etc.) before any request is in flight. + synctest.Wait() + base := bubbleNetHTTPGoroutines(t) + + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + rt := tc.roundTrip(req) + + tc.wantFrameType(FrameHeaders) + tc.writeHeaders(HeadersFrameParam{ + StreamID: rt.streamID(), + EndHeaders: true, + EndStream: false, + BlockFragment: tc.makeHeaderBlockFragment(":status", "200"), + }) + rt.wantStatus(200) + + // The request is fully sent and the response is streaming with no + // end in sight. The request-writing goroutine should be gone, + // leaving only the goroutines that predate the request. + synctest.Wait() + if n := bubbleNetHTTPGoroutines(t); n != base { + t.Errorf("got %d net/http goroutines parked during long-lived response stream; want %d (the pre-request baseline)", n, base) + } + + // The stream still works and still cleans up at END_STREAM. + tc.writeData(rt.streamID(), false, []byte("hello, ")) + tc.writeData(rt.streamID(), true, []byte("world")) + rt.wantBody([]byte("hello, world")) +} + +// bubbleNetHTTPGoroutines returns the number of goroutines in the calling +// test's synctest bubble that were created by non-test functions under +// net/http. The caller must be running in a synctest bubble. +func bubbleNetHTTPGoroutines(t *testing.T) int { + buf := make([]byte, 1<<20) + buf = buf[:runtime.Stack(buf, true)] + // The first record is the calling goroutine, whose header names the + // test's bubble: "goroutine 8 [running, synctest bubble 3]:". + head, _, _ := strings.Cut(string(buf), "\n") + _, id, ok := strings.Cut(head, ", synctest bubble ") + if !ok { + t.Fatalf("calling goroutine is not in a synctest bubble: %s", head) + } + bubble := ", synctest bubble " + strings.TrimSuffix(id, "]:") + "]:" + n := 0 + for g := range strings.SplitSeq(string(buf), "\n\n") { + header, _, _ := strings.Cut(g, "\n") + if !strings.HasSuffix(header, bubble) { + continue + } + i := strings.LastIndex(g, "\ncreated by ") + if i < 0 { + continue + } + fn, loc, _ := strings.Cut(g[i+len("\ncreated by "):], "\n") + if strings.HasPrefix(fn, "net/http") && !strings.Contains(loc, "_test.go:") { + n++ + } + } + return n +} + +// TestTransportRequestGoroutineExitsRespHeaderTimeout is like +// TestTransportRequestGoroutineExits, but with a ResponseHeaderTimeout +// configured: the timeout is enforced by a timer rather than a parked +// goroutine, and once response headers arrive the timer is disarmed and +// must not fire even long after the timeout elapses. +func TestTransportRequestGoroutineExitsRespHeaderTimeout(t *testing.T) { + synctest.Test(t, testTransportRequestGoroutineExitsRespHeaderTimeout) +} +func testTransportRequestGoroutineExitsRespHeaderTimeout(t *testing.T) { + const timeout = 1 * time.Second + tc := newTestClientConn(t, func(t1 *http.Transport) { + t1.ResponseHeaderTimeout = timeout + }) + tc.greet() + + // Count the goroutines net/http has parked (the connection's read + // loop, etc.) before any request is in flight. + synctest.Wait() + base := bubbleNetHTTPGoroutines(t) + + req, _ := http.NewRequest("GET", "https://dummy.tld/", nil) + rt := tc.roundTrip(req) + + tc.wantFrameType(FrameHeaders) + + // The request-writing goroutine should be gone even before response + // headers arrive; the response header timeout is enforced by a timer. + synctest.Wait() + if n := bubbleNetHTTPGoroutines(t); n != base { + t.Errorf("got %d net/http goroutines parked awaiting response headers; want %d (the pre-request baseline)", n, base) + } + + // Response headers arrive within the timeout. + time.Sleep(timeout / 2) + tc.writeHeaders(HeadersFrameParam{ + StreamID: rt.streamID(), + EndHeaders: true, + EndStream: false, + BlockFragment: tc.makeHeaderBlockFragment(":status", "200"), + }) + rt.wantStatus(200) + + // Long after the response header timeout has elapsed, the + // still-streaming response must be unaffected. + time.Sleep(10 * timeout) + synctest.Wait() + tc.writeData(rt.streamID(), true, []byte("hello")) + rt.wantBody([]byte("hello")) +} diff --git a/src/net/http/server.go b/src/net/http/server.go index aff62f9a8dc595..f2116101af70c2 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -23,6 +23,7 @@ import ( "net/textproto" "net/url" urlpkg "net/url" + "os" "path" "runtime" "slices" @@ -782,6 +783,46 @@ func (cr *connReader) backgroundRead() { cr.cond.Broadcast() } +// idleBufsReleaseDelay is how long a keep-alive connection waits for +// its next request before it is considered idle and its bufio buffers +// are released to their pools. It trades a little extra work on +// connections that idle past it against pinning ~8 kB of buffers on +// every waiting connection. +const idleBufsReleaseDelay = 50 * time.Millisecond + +// waitReadable blocks until data arrives on the connection, stashing +// the byte it reads for the next connReader.Read, and reports whether +// data arrived. It is called between requests, after the connection's +// bufio buffers have been released to their pools, so that an idle +// connection pins no buffer memory while it waits, possibly for a long +// time, for the next request. A false return means the read failed +// (EOF, a timeout, or another error) and the error has been handled by +// handleReadErrorLocked. +func (cr *connReader) waitReadable() (readable bool) { + cr.lock() + if cr.inRead { + panic("invalid concurrent connReader.waitReadable call") + } + if cr.hasByte { + cr.unlock() + return true + } + cr.inRead = true + cr.unlock() + n, err := cr.rwc.Read(cr.byteBuf[:]) + cr.lock() + cr.inRead = false + if n == 1 { + cr.hasByte = true + } + if err != nil { + cr.handleReadErrorLocked(err) + } + cr.unlock() + cr.cond.Broadcast() + return n == 1 && err == nil +} + func (cr *connReader) abortPendingRead() { cr.lock() defer cr.unlock() @@ -1210,6 +1251,9 @@ func (w *response) WriteHeader(code int) { w.conn.server.logf("http: superfluous response.WriteHeader call from %s (%s:%d)", caller.Function, path.Base(caller.File), caller.Line) return } + if responseWriteEnforcer != nil { + responseWriteEnforcer(w.req) + } checkWriteHeaderCode(code) // Sending a 100 Continue or any non-1XX header disables the @@ -2160,17 +2204,49 @@ func (c *conn) serve(ctx context.Context) { return } + var idleDeadline time.Time if d := c.server.idleTimeout(); d > 0 { - c.rwc.SetReadDeadline(time.Now().Add(d)) - } else { - c.rwc.SetReadDeadline(time.Time{}) + idleDeadline = time.Now().Add(d) } // Wait for the connection to become readable again before trying to // read the next request. This prevents a ReadHeaderTimeout or // ReadTimeout from starting until the first bytes of the next request // have been received. - if _, err := c.bufr.Peek(4); err != nil { + // + // The wait runs in two phases. First wait briefly with the + // connection's bufio buffers still attached: on a busy + // connection the next request typically arrives almost + // immediately, and this keeps the buffer release below off the + // hot path. If the connection then still has nothing buffered, + // it has gone idle, possibly for a long time, so release its + // bufio.Reader and Writer (~8 kB of per-connection memory + // holding no data) to their pools for the rest of the wait. + // The byte read by waitReadable is stashed in the connReader + // and yielded by its next Read after fresh buffers are + // acquired. + shortDeadline := time.Now().Add(idleBufsReleaseDelay) + if !idleDeadline.IsZero() && idleDeadline.Before(shortDeadline) { + shortDeadline = idleDeadline + } + c.rwc.SetReadDeadline(shortDeadline) + _, peekErr := c.bufr.Peek(4) + if errors.Is(peekErr, os.ErrDeadlineExceeded) && (idleDeadline.IsZero() || time.Now().Before(idleDeadline)) { + c.rwc.SetReadDeadline(idleDeadline) + if c.bufr.Buffered() == 0 && c.bufw.Buffered() == 0 { + putBufioReader(c.bufr) + c.bufr = nil + putBufioWriter(c.bufw) + c.bufw = nil + if !c.r.waitReadable() { + return + } + c.bufr = newBufioReader(c.r) + c.bufw = newBufioWriterSize(checkConnErrorWriter{c}, 4<<10) + } + _, peekErr = c.bufr.Peek(4) + } + if peekErr != nil { return } diff --git a/src/net/http/tailscale.go b/src/net/http/tailscale.go new file mode 100644 index 00000000000000..ceaceb9db8f609 --- /dev/null +++ b/src/net/http/tailscale.go @@ -0,0 +1,58 @@ +// Copyright 2023 Tailscale. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http + +var roundTripEnforcer func(*Request) error + +var responseWriteEnforcer func(*Request) + +// SetRoundTripEnforcer sets a program-global resolver enforcer that can cause +// RoundTrip calls to fail based on the request and its context. +// +// f must be non-nil. +// +// SetRoundTripEnforcer can only be called once, and must not be called +// concurrent with any RoundTrip call; it's expected to be registered during +// init. +func SetRoundTripEnforcer(f func(*Request) error) { + if f == nil { + panic("nil func") + } + if roundTripEnforcer != nil { + panic("already called") + } + roundTripEnforcer = f +} + +// SetResponseWriteEnforcer sets a program-global response writing enforcer +// which can panic to interrupt the sending of a response. +// +// f must be non-nil. +// +// SetResponseWriteEnforcer can only be called once, and must not be called +// concurrent with any ResponseWriter.WriteHeader call; it's expected to be +// registered during init. +// +// f only accepts a *Request - and not the ResponseWriter - because the +// HTTP/2 responseWriterState is free to write its own status line and has no +// reference to the responseWriter. +func SetResponseWriteEnforcer(f func(*Request)) { + if f == nil { + panic("nil func") + } + if responseWriteEnforcer != nil { + panic("already called") + } + responseWriteEnforcer = f + if registerHTTP2ResponseWriteEnforcer != nil { + registerHTTP2ResponseWriteEnforcer(f) + } +} + +// registerHTTP2ResponseWriteEnforcer installs the enforcer into the +// net/http/internal/http2 package. +// It is set during init by tailscale_http2.go, unless the nethttpomithttp2 +// build tag is set. +var registerHTTP2ResponseWriteEnforcer func(f func(*Request)) diff --git a/src/net/http/tailscale_http2.go b/src/net/http/tailscale_http2.go new file mode 100644 index 00000000000000..95c85fd9458a4c --- /dev/null +++ b/src/net/http/tailscale_http2.go @@ -0,0 +1,41 @@ +// Copyright 2026 Tailscale. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +//go:build !nethttpomithttp2 + +package http + +import ( + "net/http/internal/http2" +) + +func init() { + registerHTTP2ResponseWriteEnforcer = func(f func(*Request)) { + http2.ResponseWriteEnforcer = func(sr *http2.ServerRequest) { + f(requestFromHTTP2ServerRequest(sr)) + } + } +} + +// requestFromHTTP2ServerRequest converts an http2.ServerRequest to a Request, +// mirroring the conversion done by http2Handler.ServeHTTP in http2.go. +func requestFromHTTP2ServerRequest(sr *http2.ServerRequest) *Request { + return &Request{ + ctx: sr.Context, + Proto: "HTTP/2.0", + ProtoMajor: 2, + ProtoMinor: 0, + Method: sr.Method, + URL: sr.URL, + Header: Header(sr.Header), + RequestURI: sr.RequestURI, + Trailer: Header(sr.Trailer), + Body: sr.Body, + Host: sr.Host, + ContentLength: sr.ContentLength, + RemoteAddr: sr.RemoteAddr, + TLS: sr.TLS, + MultipartForm: sr.MultipartForm, + } +} diff --git a/src/net/http/tailscale_test.go b/src/net/http/tailscale_test.go new file mode 100644 index 00000000000000..f31d7619711c7f --- /dev/null +++ b/src/net/http/tailscale_test.go @@ -0,0 +1,104 @@ +// Copyright 2023 Tailscale. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http_test + +import ( + "io" + "log" + "net/http" + . "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +func init() { + SetResponseWriteEnforcer(func(r *http.Request) { + respEnf.mu.Lock() + fs := slices.Clone(respEnf.fs) + respEnf.mu.Unlock() + + for _, f := range fs { + if f != nil { + f(r) + } + } + }) +} + +var respEnf = struct { + mu sync.Mutex + fs []func(*Request) +}{} + +func addResponseWriteEnforcer(f func(*Request)) (cleanup func()) { + respEnf.mu.Lock() + defer respEnf.mu.Unlock() + n := len(respEnf.fs) + respEnf.fs = append(respEnf.fs, f) + + return func() { + respEnf.mu.Lock() + defer respEnf.mu.Unlock() + respEnf.fs[n] = nil + } +} + +func TestSetResponseWriteEnforcer(t *testing.T) { + run(t, testSetResponseWriteEnforcer, testNotParallel) +} +func testSetResponseWriteEnforcer(t *testing.T, mode testMode) { + var found atomic.Bool + remove := addResponseWriteEnforcer(func(r *Request) { + t.Logf("addResponseWriteEnforcer: %s %s", r.Method, r.URL) + if r.URL.Query().Get("t") == t.Name() { + found.Store(true) + } + }) + defer remove() + + errLog := new(strings.Builder) + defer func() { + if t.Failed() && errLog.Len() > 0 { + t.Logf("error logs:\n%s", errLog) + } + }() + + cst := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) { + if !r.URL.Query().Has("noop") { + io.WriteString(w, t.Name()) + } + }), + func(ts *httptest.Server) { + ts.Config.ReadTimeout = 250 * time.Millisecond + ts.Config.ErrorLog = log.New(errLog, "", 0) + }, + ) + ts := cst.ts + client := ts.Client() + + for _, q := range []string{"", "&noop=1"} { + found.Store(false) + + u := ts.URL + "?t=" + url.QueryEscape(t.Name()) + q + t.Log("GET", u) + res, err := client.Get(u) + if err != nil { + t.Fatalf("unexpected error making request: %s", err) + } + b, err := httputil.DumpResponse(res, true) + t.Logf("Response: error(%v)%s", err, b) + + if !found.Load() { + t.Errorf("request %s did not reach SetResponseWriteEnforcer callback", q) + } + } +} diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 695f89b8cd2b8b..90db79b4882c0f 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -609,6 +609,12 @@ func validateHeaders(hdrs Header) string { // roundTrip implements a RoundTripper over HTTP. func (t *Transport) roundTrip(req *Request) (_ *Response, err error) { + if roundTripEnforcer != nil { + if err := roundTripEnforcer(req); err != nil { + return nil, err + } + } + t.nextProtoOnce.Do(t.onceSetNextProtoDefaults) ctx := req.Context() trace := httptrace.ContextClientTrace(ctx) diff --git a/src/net/sock_posix.go b/src/net/sock_posix.go index cc812e3137b5ca..79cfb63cdcc6fb 100644 --- a/src/net/sock_posix.go +++ b/src/net/sock_posix.go @@ -25,6 +25,21 @@ func socket(ctx context.Context, net string, family, sotype, proto int, ipv6only return nil, err } fd = newFD(s, family, sotype, net) + if trace := ContextSockTrace(ctx); trace != nil { + fd.readHook = trace.DidRead + fd.writeHook = trace.DidWrite + if (trace.DidCreateTCPConn != nil || trace.WillCloseTCPConn != nil) && len(net) >= 3 && net[0:3] == "tcp" { + c := newRawConn(fd) + if trace.DidCreateTCPConn != nil { + trace.DidCreateTCPConn(c) + } + if trace.WillCloseTCPConn != nil { + fd.closeHook = func() { + trace.WillCloseTCPConn(c) + } + } + } + } // This function makes a network file descriptor for the // following applications: diff --git a/src/net/socktrace.go b/src/net/socktrace.go new file mode 100644 index 00000000000000..b02a8d12484d4e --- /dev/null +++ b/src/net/socktrace.go @@ -0,0 +1,53 @@ +// Copyright 2023 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package net + +import ( + "context" + "syscall" +) + +// SockTrace is a set of hooks to run at various operations on a network socket. +// Any particular hook may be nil. Functions may be called concurrently from +// different goroutines. +type SockTrace struct { + // DidOpenTCPConn is called when a TCP socket was created. The + // underlying raw network connection that was created is provided. + DidCreateTCPConn func(c syscall.RawConn) + // DidRead is called after a successful read from the socket, where n bytes + // were read. + DidRead func(n int) + // DidWrite is called after a successful write to the socket, where n bytes + // were written. + DidWrite func(n int) + // WillOverwrite is called when the registered trace is overwritten by a + // subsequent call to WithSockTrace. The provided trace is the new trace + // that will be used. + WillOverwrite func(trace *SockTrace) + // WillCloseTCPConn is called when a TCP socket is about to be closed. The + // underlying raw network connection that is being closed is provided. + WillCloseTCPConn func(c syscall.RawConn) +} + +// WithSockTrace returns a new context based on the provided parent +// ctx. Socket reads and writes made with the returned context will use +// the provided trace hooks. Any previous hooks registered with ctx are +// ovewritten (their WillOverwrite hook will be called). +func WithSockTrace(ctx context.Context, trace *SockTrace) context.Context { + if previous := ContextSockTrace(ctx); previous != nil && previous.WillOverwrite != nil { + previous.WillOverwrite(trace) + } + return context.WithValue(ctx, sockTraceKey{}, trace) +} + +// ContextSockTrace returns the SockTrace associated with the +// provided context. If none, it returns nil. +func ContextSockTrace(ctx context.Context) *SockTrace { + trace, _ := ctx.Value(sockTraceKey{}).(*SockTrace) + return trace +} + +// unique type to prevent assignment. +type sockTraceKey struct{} diff --git a/src/os/pidfd_linux.go b/src/os/pidfd_linux.go index 796d8c018c7f2a..5cdbf1175e0db5 100644 --- a/src/os/pidfd_linux.go +++ b/src/os/pidfd_linux.go @@ -138,6 +138,16 @@ func (p *Process) pidfdSendSignal(s syscall.Signal) error { // pidfdWorks returns whether we can use pidfd on this system. func pidfdWorks() bool { + if runtime.GOOS == "android" { + // Tailscale-specific workaround since https://github.com/golang/go/pull/69543/commits/aad6b3b32c81795f86bc4a9e81aad94899daf520 + // does not solve https://github.com/golang/go/issues/69065 for Android apps using Go libraries. + // + // See: https://github.com/tailscale/tailscale/issues/13452 + // + // For now (2025-04-09), we'll just disable pidfd + // on all Android releases. + return false + } return checkPidfdOnce() == nil } diff --git a/src/runtime/debug/mod.go b/src/runtime/debug/mod.go index 9b3450de5609d5..92844b51091f37 100644 --- a/src/runtime/debug/mod.go +++ b/src/runtime/debug/mod.go @@ -11,6 +11,18 @@ import ( "strings" ) +const tailscaleGitRev = `TAILSCALE_GIT_REV_TO_BE_REPLACED_AT_BUILD_TIME` + +// TailscaleToolchainGitRev reports the git revision of the Tailscale Go +// toolchain, if known. CI and Nix replace the placeholder value at build +// time via sed. See .github/workflows/build.yml. +func TailscaleToolchainGitRev() (gitHash string, ok bool) { + if len(tailscaleGitRev) > 0 && tailscaleGitRev[0] != 'T' { + return tailscaleGitRev, true + } + return "", false +} + // exported from runtime. func modinfo() string @@ -34,6 +46,13 @@ func ReadBuildInfo() (info *BuildInfo, ok bool) { // awkwardness from the user. bi.GoVersion = runtime.Version() + if rev, ok := TailscaleToolchainGitRev(); ok { + bi.Settings = append(bi.Settings, BuildSetting{ + Key: "tailscale.toolchain.rev", + Value: rev, + }) + } + return bi, true } diff --git a/src/runtime/netpoll_epoll.go b/src/runtime/netpoll_epoll.go index e658cc3d89c666..d6d00e67e917ff 100644 --- a/src/runtime/netpoll_epoll.go +++ b/src/runtime/netpoll_epoll.go @@ -18,6 +18,21 @@ var ( netpollWakeSig atomic.Uint32 // used to avoid duplicate calls of netpollBreak ) +// netpollPackData stores pd and its fdseq in the epoll data field of ev as +// a taggedPointer. A nil pd marks the netpollBreak eventfd: netpollopen always +// registers a non-nil *pollDesc, so netpollUnpackData can tell the two apart +// by the pointer alone, on any word size or endianness. +func netpollPackData(ev *linux.EpollEvent, pd *pollDesc, fdseq uintptr) { + *(*taggedPointer)(unsafe.Pointer(&ev.Data)) = taggedPointerPack(unsafe.Pointer(pd), fdseq) +} + +// netpollUnpackData returns the pollDesc and fdseq stored by netpollPackData. +// pd is nil for the netpollBreak eventfd. +func netpollUnpackData(ev *linux.EpollEvent) (pd *pollDesc, fdseq uintptr) { + tp := *(*taggedPointer)(unsafe.Pointer(&ev.Data)) + return (*pollDesc)(tp.pointer()), tp.tag() +} + func netpollinit() { var errno uintptr epfd, errno = linux.EpollCreate1(linux.EPOLL_CLOEXEC) @@ -33,7 +48,7 @@ func netpollinit() { ev := linux.EpollEvent{ Events: linux.EPOLLIN, } - *(**uintptr)(unsafe.Pointer(&ev.Data)) = &netpollEventFd + netpollPackData(&ev, nil, 0) errno = linux.EpollCtl(epfd, linux.EPOLL_CTL_ADD, efd, &ev) if errno != 0 { println("runtime: epollctl failed with", errno) @@ -49,8 +64,7 @@ func netpollIsPollDescriptor(fd uintptr) bool { func netpollopen(fd uintptr, pd *pollDesc) uintptr { var ev linux.EpollEvent ev.Events = linux.EPOLLIN | linux.EPOLLOUT | linux.EPOLLRDHUP | linux.EPOLLET - tp := taggedPointerPack(unsafe.Pointer(pd), pd.fdseq.Load()) - *(*taggedPointer)(unsafe.Pointer(&ev.Data)) = tp + netpollPackData(&ev, pd, pd.fdseq.Load()) return linux.EpollCtl(epfd, linux.EPOLL_CTL_ADD, int32(fd), &ev) } @@ -137,7 +151,9 @@ retry: continue } - if *(**uintptr)(unsafe.Pointer(&ev.Data)) == &netpollEventFd { + pd, tag := netpollUnpackData(&ev) + // pd == nil denotes a netpollBreak eventfd. + if pd == nil { if ev.Events != linux.EPOLLIN { println("runtime: netpoll: eventfd ready for", ev.Events) throw("runtime: netpoll: eventfd ready for something unexpected") @@ -163,9 +179,6 @@ retry: mode += 'w' } if mode != 0 { - tp := *(*taggedPointer)(unsafe.Pointer(&ev.Data)) - pd := (*pollDesc)(tp.pointer()) - tag := tp.tag() if pd.fdseq.Load() == tag { pd.setEventErr(ev.Events == linux.EPOLLERR, tag) delta += netpollready(&toRun, pd, mode) diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go index 42af4200547b64..73e2b213f3f030 100644 --- a/src/runtime/os_windows.go +++ b/src/runtime/os_windows.go @@ -1176,6 +1176,23 @@ const preemptMSupported = true // suspending each other. var suspendLock mutex +// threadPausedLockRank warns the runtime against acquiring new locks on this +// thread while it has another thread paused. The holder of this lock rank is +// effectively a signal handler for a paused thread, which may be holding +// arbitrary locks. +// +// The warning takes two concrete forms: First, unlock2 will see that this +// thread is holding other locks, so won't try to flush the mutex contention +// buffer (which can involve acquiring another lock). Second, builds with +// GOEXPERIMENT=staticlockranking (currently broken, https://go.dev/issue/76058) +// will report a rank violation. +// +// suspendLock serializes the transition into "suspended", but it's fine for +// multiple threads to be in the suspended state concurrently, with each of +// their sponsors independently using threadPausedLockRank to discourage +// themselves from acquiring new locks. +const threadPausedLockRank = lockRankLeafRank + func preemptM(mp *m) { if mp == getg().m { throw("self-preempt") @@ -1218,6 +1235,9 @@ func preemptM(mp *m) { // actually suspended. lock(&suspendLock) + // Signal handler rules are in effect; don't acquire new locks. + acquireLockRankAndM(threadPausedLockRank) + // Suspend the thread. if int32(stdcall(_SuspendThread, thread)) == -1 { unlock(&suspendLock) @@ -1226,6 +1246,7 @@ func preemptM(mp *m) { // The thread no longer exists. This shouldn't be // possible, but just acknowledge the request. mp.preemptGen.Add(1) + releaseLockRankAndM(threadPausedLockRank) return } @@ -1260,6 +1281,8 @@ func preemptM(mp *m) { stdcall(_ResumeThread, thread) stdcall(_CloseHandle, thread) + + releaseLockRankAndM(threadPausedLockRank) } // osPreemptExtEnter is called before entering external code that may diff --git a/src/runtime/tailscale_runtime.go b/src/runtime/tailscale_runtime.go new file mode 100644 index 00000000000000..25254ddb12b86f --- /dev/null +++ b/src/runtime/tailscale_runtime.go @@ -0,0 +1,33 @@ +// Copyright 2024 Tailscale. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package runtime + +// TailscaleCurrentP returns the runtime's currently executing 'p' ID. +// +// See https://github.com/tailscale/go/issues/109. +func TailscaleCurrentP() int { + return int(getg().m.p.ptr().id) +} + +// TailscaleNumTimers returns the number of timers that are currently +// pending in the runtime across all Ps, as well as the number of "zombie" +// timers. +func TailscaleNumTimers() (total, zombies int) { + var sum uint32 + var nzombies int32 + + // Prevent allp slice changes. This is like retake. + lock(&allpLock) + for _, pp := range allp { + if pp == nil { + continue + } + sum += pp.timers.len.Load() + nzombies += pp.timers.zombies.Load() + } + unlock(&allpLock) + + return int(sum), int(nzombies) +}