Skip to content

chore: simplify and modernize for Go 1.27 - #2627

Open
dunglas wants to merge 7 commits into
mainfrom
go-1.27-modernize
Open

chore: simplify and modernize for Go 1.27#2627
dunglas wants to merge 7 commits into
mainfrom
go-1.27-modernize

Conversation

@dunglas

@dunglas dunglas commented Aug 28, 2026

Copy link
Copy Markdown
Member

Follow-up to the Go 1.27 upgrade in a411024, applying what the new release makes available.

Modernizers

go fix gained slicesbackward, embedlit and unsafefuncs, and renamed waitgroup to waitgroupgo. Applied across both modules, with two exceptions:

  • The extgen parsers are left alone: embedlit rewrites their composite literals into a shape that reads worse than the original.
  • go fix skips files that import "C", so the root package never got the transformations applied everywhere else. Those are done by hand in a separate commit.

Standard library

  • splitRemoteAddr uses strings.CutLast, new in 1.27. Its not-found return is exactly what the old else branch assigned, so the branch disappears.
  • The manual unsafe.Pointer/uintptr arithmetic in types.go and frankenphp.go becomes unsafe.Add. The old form is only valid inside a single expression and silently breaks if a later refactor splits the line.
  • errors.As becomes errors.AsType, dropping the throwaway target values.
  • Eleven copies of the register-or-panic block in metrics.go collapse into one mustRegister helper.

Tests

internal/state moves to testing/synctest. synctest.Wait returns once every subscriber goroutine is durably blocked, so the subscriber count is exact rather than polled for up to a second. This also makes the WaitForStateWithTimeout give-up path worth covering, since the bubble's fake clock fires its one second timeout instantly.

The other timing tests stay as they are. regularRequestChan is a package global that live PHP thread goroutines receive from, so a bubbled sender can have its send stolen by an out-of-bubble thread, and PHP threads are C threads, which never count as durably blocked. Bubbling them would buy flakiness, not determinism.

A new test uses the goroutineleak profile, GA in 1.27, to catch a thread, scaling ticker or watcher goroutine that outlives Shutdown with nothing left to wake it. Currently reports zero leaks across a full Init/Shutdown cycle.

/debug/pprof/goroutineleak needs no code to expose: net/http/pprof.Index dispatches profiles by name, so it is already live on the admin endpoint. Verified against a running server. It reports one leaked goroutine there, but the same one appears with no php_server configured at all, so it is Caddy/runtime baseline rather than ours.

Drive-by

internal/state had a bare import "C" with no C preamble and no calls into C, which forced a pure Go package through cgo. Removed.

No action needed

encoding/json/v2 backing encoding/json and the size-specialized allocator are both in the 1.27 baseline and active automatically. The stricter jsonv2 defaults ship only with an explicit encoding/json/v2 import, so v1 semantics are unchanged: duplicate keys still accepted, invalid UTF-8 still replaced rather than rejected.

Go 1.27 adds the slicesbackward, embedlit and unsafefuncs analyzers and
renames waitgroup to waitgroupgo.

The two extgen parsers are left untouched: embedlit rewrites their
composite literals into a shape that reads worse than the original.
go fix does not analyze files that import "C", so the root package never
received the transformations applied everywhere else.

splitRemoteAddr now uses strings.CutLast, new in Go 1.27. Its not-found
return value is exactly what the old else branch assigned, so the branch
is gone.

The manual unsafe.Pointer/uintptr arithmetic becomes unsafe.Add. The old
form is only valid inside a single expression and silently breaks if a
later refactor splits the line.
Drops the throwaway target values the errors.As calls had to allocate.
Eleven copies of the same register-or-panic block, each allocating an
AlreadyRegisteredError to match against.
The package has no C preamble and calls nothing in C, but the bare
import "C" still forced it through cgo.
synctest.Wait returns once every subscriber goroutine is durably blocked,
so the subscriber count is exact instead of polled for up to a second.

Covers the WaitForStateWithTimeout give-up path too, including that it
drops its subscriber. The bubble's fake clock makes its one second
timeout fire instantly.
Uses the goroutineleak profile, generally available since Go 1.27, to
catch a thread, scaling ticker or watcher goroutine that outlives
Shutdown with nothing left to wake it.

The count is compared against a baseline rather than zero because tests
share a process. Profile.Count only reports the previous detection cycle,
so the profile has to be written first to trigger one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants