fix(gate): a unit test in one package was deleting another package's TUN

`go test` runs package binaries CONCURRENTLY and every one of them shares the
host's network namespace. netplane.L3SlotFor is destructive by design — it
DELETES a candidate slot it finds occupied rather than waiting for it — and
netplane.removeL3Devices deletes both slots unconditionally. Two test binaries
reached those for real:

  shater/engine  l3slot_test.go calls l3RetargetForNext for its return value
  shater/apply   Applier.Teardown -> netplane.TeardownRouting -> removeL3Devices

Measured with an `ip` shim on PATH inside the gate container: apply.test issued
9 `ip link del shater-l3a` + 9 `ip link del shater-l3b` per run, engine.test one
per l3slot test — into the namespace where shater/generate's privileged tests
were holding a live TUN. From the other side that is

  post-start inbound/tun[l3-in]: starting TUN interface: find tun interface: Link not found
  no [shater-l3a shater-l3b] device exists after a successful Start

i.e. an intermittently red [2/7]/[4/7] in a package that did nothing wrong,
while [5/7] — which runs only `^TestIntegration`, so neither binary reaches the
slot code — passed the very same test seconds later. It only became visible when
iproute2 was installed into the gate container: without `ip` every slot read as
free and no deletion was ever issued.

Not a product defect. shaterd is one process with one engine; the running
generation's slot is excluded before anything is deleted, and nothing else on
the router calls L3SlotFor.

The kernel is faked rather than the CHOICE: making the engine's tests stub the
slot answer would delete the only place the ENGINE checks that the running
generation's slot is excluded, which is the invariant the production outage
violated. netplane.L3StubKernelForTest points the two kernel operations at an
in-memory set; engine and apply install it from TestMain (forget-proof, unlike a
per-test helper whose omission fails in a different package on some runs only).
netplane's TestL3StubKernelTakesTheSlotChoiceOffTheKernel is the control, in
both directions: stubbed, nothing reaches the exec seam; restored, the same call
does.

Mutation: with the engine TestMain reverted, the generate binary's
TestIntegrationL3* failed 8 of 8 runs beside a loop of the engine binary; with
it, 0 of 8. With L3StubKernelForTest degraded to a no-op, the control fails
naming the three escaped `ip` calls.

Also: the DoH3 ownership test's control now retries.
requireInstrumentFindsPackedQuery packed a query into a pooled buffer, released
it and demanded the scan find it — but under -race sync.Pool.Put drops one
object in four on purpose, so the control failed 18 of 60 measured runs and took
the whole -race pass down with it. Its sibling control in the same file already
retried for exactly this reason. The claim is existential ("this instrument CAN
find a released buffer"), so one success out of 32 proves it and nothing is
diluted; 0 of 60 after. What it does not buy is stated in the code: the VERDICT
is still a 3-in-4 detector under -race, which is the safe direction, and the
non-race pass runs the same test as a certainty.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS
This commit is contained in:
2026-07-27 03:25:46 +03:00
co-authored by Claude Opus 5
parent 3654acf7fb
commit 06c04c157d
5 changed files with 245 additions and 21 deletions
@@ -60,6 +60,31 @@ const (
markedQueryNeedle = 64
// How deep to drain a size class when looking for the needle.
poolScanDepth = 64
// How many times a CONTROL may repeat before it gives up.
//
// Both controls in this file assert the same thing — a buffer released while
// its bytes are still referenced comes back out of the pool — and under
// `-race` that is a DICE ROLL, not a certainty: sync.Pool.Put drops one
// object in four on purpose (runtime_randn(4) == 0, sync/pool.go). Measured
// in golang:1.26 with `go test -race -count=60`: the single-attempt control
// failed 18 times out of 60, i.e. the gate's -race pass had a ~30% chance of
// going red on a tree with nothing wrong with it.
//
// A retry is the honest repair rather than a papering-over, because the
// control's claim is EXISTENTIAL — "this instrument is able to find a
// released, still-referenced buffer" — and one success proves it. It is not
// an average over attempts, so nothing is diluted by taking more than one.
// 32 attempts leave a (1/4)^32 chance of a false alarm.
//
// What this does NOT do, said plainly: it does not make the VERDICT below
// certain under -race. The same 1-in-4 drop means a scan that comes back
// clean has a 1-in-4 chance of being clean because the pool threw the
// evidence away. That direction is the safe one — it can only let a broken
// build look clean, never make a clean build look broken — and the -race
// pass is not the only one that runs this test: [2/7] of scripts/run-tests.sh
// runs the same file WITHOUT -race, where both the control and the verdict
// are certainties.
controlAttempts = 32
)
func paddedQuery(t *testing.T) (*mDNS.Msg, []byte) {
@@ -115,10 +140,10 @@ func releaseAll(buffers []*buf.Buffer) {
// Retried, because under -race sync.Pool.Put drops one object in four on
// purpose. That same dice roll is why the check below is a 3-in-4 detector under
// -race and a certainty without it; it can only make a broken build look clean,
// never a clean build look broken.
// never a clean build look broken. See controlAttempts.
func requirePoisonReachesReleasedBuffer(t *testing.T, size int, pattern []byte) {
t.Helper()
for range 32 {
for range controlAttempts {
control := buf.NewSize(size)
free := control.FreeBytes()
if len(free) < len(pattern) {
@@ -281,24 +306,38 @@ func poolHoldsNeedle(size int, needle []byte, count int) bool {
// Exchange did — pack a query into a pooled buffer and release it — and demands
// that the scan below FINDS the needle. Without it, "the pool does not hold the
// query" would also be the verdict for a scan that can never find anything.
//
// Retried for the same reason its sibling control above is, and it was NOT
// before: under -race sync.Pool.Put drops one object in four, so a single
// attempt made this control — and with it the whole -race pass of the gate —
// fail on 18 of 60 measured runs with nothing wrong in the tree. A fresh
// needle is packed on each attempt, so a later one cannot be answered by an
// earlier one's bytes. See controlAttempts for what the retry does and does not
// buy.
func requireInstrumentFindsPackedQuery(t *testing.T) {
t.Helper()
message, needle := markedQuery(t)
size := 1 + message.Len()
exMessage := *message
exMessage.Id = 0
exMessage.Compress = true
for range controlAttempts {
message, needle := markedQuery(t)
size := 1 + message.Len()
exMessage := *message
exMessage.Id = 0
exMessage.Compress = true
buffer := buf.NewSize(size)
if _, err := exMessage.PackBuffer(buffer.FreeBytes()); err != nil {
t.Fatal(err)
}
buffer.Release()
buffer := buf.NewSize(size)
if _, err := exMessage.PackBuffer(buffer.FreeBytes()); err != nil {
t.Fatal(err)
}
buffer.Release()
if !poolHoldsNeedle(size, needle, poolScanDepth) {
t.Fatal("control failed: a query packed into a pooled buffer and released was NOT found by the scan, " +
"so a clean verdict below would prove nothing")
if poolHoldsNeedle(size, needle, poolScanDepth) {
return
}
}
t.Fatalf("control failed: %d times in a row, a query packed into a pooled buffer and released was NOT "+
"found by the scan, so a clean verdict below would prove nothing. Under -race sync.Pool.Put drops "+
"one object in four, which is what the retries absorb; this many consecutive misses is something "+
"else — a pool that zeroes on Put, a size class that stopped being pooled, or buf.Buffer no longer "+
"handing its array back at all", controlAttempts)
}
// TestHTTP3ExchangeNeverPacksQueriesIntoPooledMemory pins the ownership rule the
+31
View File
@@ -2,6 +2,7 @@ package apply
import (
"errors"
"os"
"strings"
"testing"
"time"
@@ -12,6 +13,36 @@ import (
"github.com/sagernet/sing-box/shater/netplane"
)
// TestMain takes THIS TEST BINARY off the real network namespace, for the one
// operation of netplane's that destroys something another test binary can be
// using: the L3-ingress TUN devices.
//
// Applier.Teardown calls netplane.TeardownRouting for real here, and its
// device sweep deletes BOTH slots unconditionally. `go test` runs package
// binaries concurrently (-p defaults to GOMAXPROCS) and they all share one
// network namespace, so on a runner with iproute2 and /dev/net/tun this binary
// was issuing 9 `ip link del shater-l3a` and 9 `ip link del shater-l3b` per
// gate run — measured with an `ip` shim on PATH — into the namespace where
// shater/generate's privileged tests hold a live TUN. What that looks like from
// the other side is a red TestIntegrationL3* in a package that did nothing
// wrong:
//
// no [shater-l3a shater-l3b] device exists after a successful Start
//
// netplane.L3StubKernelForTest carries the full measurement and the reasoning.
//
// Scope, stated rather than implied: this diverts ONLY the L3 device deletes.
// The `ip rule del` / `ip route flush` on the reserved tables that teardown also
// performs still run for real from here. They are left alone because nothing
// else in the gate reads those tables, so unlike the devices they have no
// observed victim — not because they are harmless on a machine that matters.
func TestMain(m *testing.M) {
restore := netplane.L3StubKernelForTest()
code := m.Run()
restore()
os.Exit(code)
}
// TestCanRollback pins the signal the panel gates its rollback control on:
// canRollback is false on a fresh applier (no armed commit-confirm snapshot AND
// the engine holds no last-good predecessor), and flips to true once a snapshot
+37
View File
@@ -6,6 +6,7 @@ package engine
// back into the options the hash gate compares.
import (
"os"
"testing"
C "github.com/sagernet/sing-box/constant"
@@ -14,6 +15,42 @@ import (
"github.com/sagernet/sing-box/shater/netplane"
)
// TestMain takes THIS TEST BINARY off the real network namespace.
//
// The tests below call l3RetargetForNext for its return value, and that reaches
// netplane.L3SlotFor — which does not merely ask the kernel about a device, it
// DELETES one it finds occupying a candidate slot. `go test` runs package
// binaries concurrently (-p defaults to GOMAXPROCS) and every one of them shares
// the host's network namespace, so on a runner that has iproute2 and a real
// /dev/net/tun these unit tests were deleting the TUN device shater/generate's
// privileged tests had just opened:
//
// post-start inbound/tun[l3-in]: starting TUN interface: find tun interface: Link not found
//
// which is a red gate in a package that did nothing wrong. netplane.L3StubKernelForTest
// carries the measurement and the reasoning; netplane's own
// TestL3StubKernelTakesTheSlotChoiceOffTheKernel is the control that the hook
// still diverts.
//
// The fake kernel starts EMPTY, so every slot reads as free and no reclaim is
// ever attempted from here. That costs this file nothing: the reclaim is
// netplane's subject (TestL3SlotForReclaimsARetiredSlot, against netplane's own
// exec fake), the live-kernel proof is shater/generate's
// TestIntegrationL3StaleSlotIsReclaimed, and what the tests below are about —
// that the slot handed to the next generation is never the running one, and
// that the substitution does not leak into the canonical options — is answered
// by the real L3SlotFor either way.
//
// It is a TestMain rather than a per-test helper deliberately: a helper is
// something the next test added here can forget, and the failure that causes
// lands in a DIFFERENT package, on some runs only.
func TestMain(m *testing.M) {
restore := netplane.L3StubKernelForTest()
code := m.Run()
restore()
os.Exit(code)
}
// l3Opts is a config carrying one L3-ingress TUN inbound exactly as generate
// emits it: the canonical placeholder name, which is the only name the retarget
// is allowed to recognise.
+76 -6
View File
@@ -154,19 +154,82 @@ func L3SlotFor(current string) string {
}
// current matches at most one slot, so candidates is never empty.
for _, s := range candidates {
if !deviceExists(s) {
if !l3Present(s) {
return s
}
}
for _, s := range candidates {
_ = execCommand("ip", "link", "del", s).Run()
if !deviceExists(s) {
l3Delete(s)
if !l3Present(s) {
return s
}
}
return candidates[0]
}
// l3Present and l3Delete are the two things the slot choice above does to the
// kernel: ask whether a device is there, and take it back. They are variables
// only so that a test in ANOTHER package can point them at memory —
// L3StubKernelForTest is where that is done and why.
//
// The defaults are the real operations, so nothing about the daemon or about
// this package's own tests (which intercept one level lower, at execCommand)
// changes: l3Present is deviceExists, and l3Delete is the same `ip link del`
// removeL3Devices issues.
var (
l3Present = func(dev string) bool { return deviceExists(dev) }
l3Delete = func(dev string) { _ = execCommand("ip", "link", "del", dev).Run() }
)
// L3StubKernelForTest makes L3SlotFor answer from an in-memory set of device
// names instead of the real network namespace, and returns the function that
// puts the kernel back. present is what the fake kernel starts with; a reclaim
// deletes from it, exactly as `ip link del` would.
//
// # Why product code carries a test hook
//
// L3SlotFor is DESTRUCTIVE by design: a candidate slot that is still present is
// deleted rather than waited for. That is right for the daemon — one process,
// one engine, and any slot other than the running generation's belongs to a
// generation this same process has already retired.
//
// It is wrong for a UNIT TEST, and that is measured rather than feared.
// shater/engine's l3slot tests call l3RetargetForNext for its RETURN VALUE, but
// `go test` runs package binaries CONCURRENTLY (-p defaults to GOMAXPROCS) and
// all of them share one network namespace. So the engine's unit tests were
// issuing a real `ip link del shater-l3a` against the TUN device
// shater/generate's privileged tests had just opened, and the gate's [2/7] and
// [4/7] passes went red intermittently with
//
// post-start inbound/tun[l3-in]: starting TUN interface: find tun interface: Link not found
//
// while [5/7], which runs only `^TestIntegration` and therefore never reaches
// this function from the engine binary, passed the very same test seconds
// later. Reproduced on demand in golang:1.26 (iproute2 present, --cap-add
// NET_ADMIN --device /dev/net/tun) by looping the engine test binary's
// `-test.run ^TestL3` beside the generate binary's `-test.run ^TestIntegrationL3`:
// 3 of 6 runs red, 0 of 6 with the binaries run apart.
//
// The alternative was to let the engine's tests fake the CHOICE instead of the
// kernel. That would have deleted the only place the ENGINE checks that the
// running generation's slot is excluded — the invariant whose absence took the
// production LAN down — so the kernel is faked and the algorithm under test
// stays the real one.
//
// This changes NOTHING on the router, and is not claimed to: shaterd is one
// process with one engine, nothing else calls L3SlotFor, and the running
// generation's slot is excluded before any deletion is considered.
func L3StubKernelForTest(present ...string) func() {
have := make(map[string]bool, len(present))
for _, dev := range present {
have[dev] = true
}
prevPresent, prevDelete := l3Present, l3Delete
l3Present = func(dev string) bool { return have[dev] }
l3Delete = func(dev string) { delete(have, dev) }
return func() { l3Present, l3Delete = prevPresent, prevDelete }
}
// l3LiveMu guards l3Live.
var l3LiveMu sync.Mutex
@@ -250,11 +313,18 @@ func deviceExists(dev string) bool {
// past (the same reason TeardownRouting sweeps the whole egress block). Failures
// are ignored — the overwhelmingly common one is "Cannot find device", which is
// the desired end state.
//
// It deletes through the same l3Delete seam L3SlotFor uses, so that a test
// binary which has installed L3StubKernelForTest cannot reach the kernel from
// here either. That is not symmetry for its own sake: this function deletes
// BOTH slots unconditionally, so from a unit test it is the more destructive of
// the two — measured, `shater/apply`'s binary issued 9 `ip link del shater-l3a`
// and 9 `ip link del shater-l3b` per gate run, in the same shared namespace
// shater/generate's privileged tests were holding a live TUN in.
func removeL3Devices() {
run := func(args ...string) { _ = execCommand("ip", args...).Run() }
for _, s := range L3Slots {
run("link", "del", s)
l3Delete(s)
}
run("link", "del", L3DeviceBase)
l3Delete(L3DeviceBase)
RememberL3Device("")
}
+47
View File
@@ -137,6 +137,53 @@ func TestL3SlotForDoesNotDeleteWhatIsAlreadyFree(t *testing.T) {
}
}
// TestL3StubKernelTakesTheSlotChoiceOffTheKernel is the CONTROL for the hook
// other packages' tests stand on (L3StubKernelForTest), and it is a control in
// both directions: it shows that with the stub installed NOTHING reaches the
// exec seam, and that with it restored the very same call does.
//
// Only the second half would be missed by an ordinary test, and it is the half
// that matters: if the hook silently stopped diverting, shater/engine's unit
// tests would go straight back to running `ip link del shater-l3a` in the
// namespace shater/generate's privileged tests are using — and the damage lands
// in ANOTHER package, on some runs only, which is exactly how this cost a day.
func TestL3StubKernelTakesTheSlotChoiceOffTheKernel(t *testing.T) {
// The exec fake is the TRIPWIRE here, not the instrument: any `ip` that
// escapes the stub is recorded in f.calls.
f := &egressPlaneNet{link: map[string]string{
L3Slots[0]: "9: " + L3Slots[0] + ": <UP>",
L3Slots[1]: "10: " + L3Slots[1] + ": <UP>",
}}
f.install(t)
restore := L3StubKernelForTest(L3Slots[1])
// Restoring twice is a no-op (it reassigns the same two functions), so the
// cleanup can stand next to the explicit restore below — and a t.Fatalf in
// between cannot leave the rest of this binary stubbed.
t.Cleanup(restore)
// current = slot 0, so slot 1 is the only candidate and the fake kernel says
// it is occupied: the reclaim must run, and must run through the stub.
if got := L3SlotFor(L3Slots[0]); got != L3Slots[1] {
t.Fatalf("L3SlotFor(%q) = %q, want %q — the stub kernel was not consulted at all", L3Slots[0], got, L3Slots[1])
}
if len(f.calls) != 0 {
t.Fatalf("the slot choice reached the real exec seam while stubbed: %v.\n"+
"Every one of those runs against the host's network namespace, which `go test` shares between "+
"the package binaries it runs concurrently — this is the `find tun interface: Link not found` "+
"that shater/generate's privileged tests were failing with", f.calls)
}
// The other direction: restored, the same call must go back to the kernel.
// Without this, a hook that diverted PERMANENTLY (or a stub that was never
// installed) would read exactly like a pass above.
restore()
_ = L3SlotFor(L3Slots[0])
if !containsCall(f.calls, "ip link show dev "+L3Slots[1]) {
t.Errorf("after restore the slot choice still did not ask the kernel (calls: %v) — this test's clean "+
"result above would then say nothing about the stub", f.calls)
}
}
// TestDeviceExistsTreatsEmptyOutputAsAbsent. `ip link show dev X` exits non-zero
// for an unknown device on a real system, but reading "no output" as "present"
// would be the fail-OPEN direction here: L3SlotFor would believe every slot is