mirror of
https://github.com/zap-proto/zip.git
synced 2026-10-19 15:38:57 +00:00
a socket path that cannot bind says so, instead of dying as EINVAL
sun_path is a fixed field holding a terminated string, so a unix address has 103 usable bytes on Darwin and 107 on Linux. Over that the kernel refuses the bind with EINVAL, which surfaces as "invalid argument" and names neither the length nor the path — the plugin child exits 1 and the host reports "exited before listening", which reads like a timing bug and is not. The limit now lives in internal/sock beside the path shape, since both are properties of the same address: minting one is not enough, it also has to be bindable. start() checks the longest address in play — the child's .sock.http upgrade listener — and refuses to launch rather than watching it die. 103 is measured, not read off a header: on Darwin a 103-byte path binds and a 104-byte one does not. The first cut of this used 104 and let through a path that was exactly one byte too long. The private instance directory also stops repeating the plugin's name. The binary and the socket inside it are already named for the plugin, so the copy in the directory spent len(name)+1 of a 103-byte budget saying it twice, and that was what pushed a long name over. Four idle tests and the status test built their Plugin.Dir from t.TempDir, which spends 89 bytes on the test's own name; sockDir has existed for exactly this reason. shortDir was a third copy of sockDir in the same test package, written from a comment that blamed sockDir for t.TempDir's length. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -129,6 +129,11 @@ func SocketPath(name string) string { return sock.Path(name) }
|
||||
// service's socket is <dir>/<name>.sock" is stated once.
|
||||
func socketIn(dir, name string) string { return sock.In(dir, name) }
|
||||
|
||||
// socketFits is the length rule, kept beside the shape rule because both are
|
||||
// properties of the same address: minting one is not enough, it also has to be
|
||||
// bindable, and whoever mints it is the only one positioned to say so.
|
||||
func socketFits(path string) error { return sock.Fits(path) }
|
||||
|
||||
// installCallPlane mounts the by-name op plane when there are ops to call.
|
||||
// Called from prepare() alongside installOpenAPIRoutes and installMCP.
|
||||
func (a *App) installCallPlane() {
|
||||
|
||||
+1
-1
@@ -31,7 +31,7 @@ func TestIdle_ReclaimsRealMemory(t *testing.T) {
|
||||
name := fmt.Sprintf("demo%d", i)
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{
|
||||
Name: name, Bin: bin, Dir: t.TempDir(), Lazy: true,
|
||||
Name: name, Bin: bin, Dir: sockDir(t), Lazy: true,
|
||||
IdleAfter: 50 * time.Millisecond,
|
||||
}, "/v1/"+name,
|
||||
)))
|
||||
|
||||
+4
-4
@@ -19,7 +19,7 @@ func TestIdle_EvictsAndComesBack(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{
|
||||
Name: "demo", Bin: bin, Dir: t.TempDir(), Lazy: true,
|
||||
Name: "demo", Bin: bin, Dir: sockDir(t), Lazy: true,
|
||||
IdleAfter: 50 * time.Millisecond,
|
||||
}, "/v1/demo",
|
||||
)))
|
||||
@@ -75,7 +75,7 @@ func TestIdle_ZeroNeverEvicts(t *testing.T) {
|
||||
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{Name: "demo", Bin: bin, Dir: t.TempDir(), Lazy: true}, // IdleAfter unset
|
||||
zip.Plugin{Name: "demo", Bin: bin, Dir: sockDir(t), Lazy: true}, // IdleAfter unset
|
||||
"/v1/demo",
|
||||
)))
|
||||
defer func() { _ = app.Shutdown() }()
|
||||
@@ -103,7 +103,7 @@ func TestIdle_EagerIsNeverEvicted(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{
|
||||
Name: "demo", Bin: bin, Dir: t.TempDir(), // Lazy false
|
||||
Name: "demo", Bin: bin, Dir: sockDir(t), // Lazy false
|
||||
IdleAfter: 10 * time.Millisecond,
|
||||
}, "/v1/demo",
|
||||
)))
|
||||
@@ -137,7 +137,7 @@ func TestIdle_ReapRunsAndStops(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{
|
||||
Name: "demo", Bin: bin, Dir: t.TempDir(), Lazy: true,
|
||||
Name: "demo", Bin: bin, Dir: sockDir(t), Lazy: true,
|
||||
IdleAfter: 20 * time.Millisecond,
|
||||
}, "/v1/demo",
|
||||
)))
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
package sock
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
)
|
||||
@@ -50,3 +51,28 @@ func Path(name string) string { return In(Dir(), name) }
|
||||
// child's private socket with it too, so "a service's socket is
|
||||
// <dir>/<name>.sock" has one definition.
|
||||
func In(dir, name string) string { return filepath.Join(dir, name+".sock") }
|
||||
|
||||
// Max is how long a socket path may be. sun_path is a fixed-size field inside
|
||||
// the address struct — 104 bytes on Darwin, 108 on Linux — and it holds a
|
||||
// TERMINATED string, so the usable length is one less than the field: 103 and
|
||||
// 107. The smaller is the one number here, so an address that binds on Darwin
|
||||
// binds on Linux.
|
||||
//
|
||||
// Measured, not read off a header: on Darwin a 103-byte path binds and a
|
||||
// 104-byte path fails. Off by one here is not a rounding error — it is a
|
||||
// plugin that starts everywhere except the one machine whose temp directory is
|
||||
// a byte longer.
|
||||
const Max = 103
|
||||
|
||||
// Fits reports whether a path is short enough to bind, naming the overrun when
|
||||
// it is not. The kernel's own answer is EINVAL, which arrives as "invalid
|
||||
// argument" and mentions neither the length nor the path — so the first guess
|
||||
// is always permissions, and the directory looks fine, and the real cause is a
|
||||
// deep parent directory nobody thought to count.
|
||||
func Fits(path string) error {
|
||||
if len(path) <= Max {
|
||||
return nil
|
||||
}
|
||||
return fmt.Errorf("socket path is %d bytes, %d over the %d-byte limit: %s",
|
||||
len(path), len(path)-Max, Max, path)
|
||||
}
|
||||
|
||||
@@ -610,7 +610,12 @@ func start(spec Plugin) (*instance, error) {
|
||||
// One private directory per instance holds the extracted binary and the
|
||||
// socket, so a reload's new instance never collides with the old one's
|
||||
// socket path and cleanup is a single RemoveAll.
|
||||
dir, err := os.MkdirTemp(spec.Dir, "zip-"+spec.Name+"-")
|
||||
//
|
||||
// The directory does not repeat the plugin's name. Everything inside it is
|
||||
// already named for the plugin — the binary and the socket both — and the
|
||||
// address they sit at has only 103 bytes total, so spending len(name)+1 of
|
||||
// them on a second copy is what pushes a long name over the limit.
|
||||
dir, err := os.MkdirTemp(spec.Dir, "zip-")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -634,6 +639,14 @@ func start(spec Plugin) (*instance, error) {
|
||||
}
|
||||
|
||||
sock := socketIn(dir, spec.Name)
|
||||
// The child opens sock+".http" for the upgrade listener too, so that is the
|
||||
// longest address in play and the one that has to fit. Checking here turns a
|
||||
// child that dies with "bind: invalid argument" into a host that refuses to
|
||||
// launch it and says why.
|
||||
if err := socketFits(sock + ".http"); err != nil {
|
||||
_ = os.RemoveAll(dir)
|
||||
return nil, err
|
||||
}
|
||||
cmd := exec.Command(bin, spec.Args...)
|
||||
cmd.Env = append(append(os.Environ(), spec.Env...), AddrEnv+"="+sock)
|
||||
// The child's output is the operator's only window into it, so it goes where
|
||||
|
||||
+4
-20
@@ -1,28 +1,12 @@
|
||||
package zip_test
|
||||
|
||||
import (
|
||||
"os"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/zap-proto/zip"
|
||||
)
|
||||
|
||||
// shortDir is sockDir(t) with a SHORT name. A plugin's socket lives inside
|
||||
// Plugin.Dir, and a unix socket path is capped at 108 bytes by the kernel —
|
||||
// sockDir(t) spends most of that budget on the test's own name, so a
|
||||
// descriptively-named test silently fails to bind and reports the child as
|
||||
// "exited before listening".
|
||||
func shortDir(t *testing.T) string {
|
||||
t.Helper()
|
||||
d, err := os.MkdirTemp("", "zs")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = os.RemoveAll(d) })
|
||||
return d
|
||||
}
|
||||
|
||||
// A lazy plugin has exactly ONE trigger today: a request reaching one of its
|
||||
// prefixes. A host that reaches its plugins another way — Hanzo's fleet calls
|
||||
// app-to-app over each app's own unix socket, which never touches the router —
|
||||
@@ -32,7 +16,7 @@ func shortDir(t *testing.T) string {
|
||||
func TestStart_BringsUpALazyPluginThatNoRequestHasReached(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: shortDir(t), Lazy: true}, "/v1/demo",
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: sockDir(t), Lazy: true}, "/v1/demo",
|
||||
)))
|
||||
t.Cleanup(func() { _ = app.Shutdown() })
|
||||
|
||||
@@ -58,7 +42,7 @@ func TestStart_BringsUpALazyPluginThatNoRequestHasReached(t *testing.T) {
|
||||
func TestStart_OnARunningPluginIsANoOp(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: shortDir(t), Lazy: true}, "/v1/demo",
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: sockDir(t), Lazy: true}, "/v1/demo",
|
||||
)))
|
||||
t.Cleanup(func() { _ = app.Shutdown() })
|
||||
|
||||
@@ -86,7 +70,7 @@ func TestStart_OnARunningPluginIsANoOp(t *testing.T) {
|
||||
func TestStart_ConcurrentFirstCallersProduceOneChild(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: shortDir(t), Lazy: true}, "/v1/demo",
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: sockDir(t), Lazy: true}, "/v1/demo",
|
||||
)))
|
||||
t.Cleanup(func() { _ = app.Shutdown() })
|
||||
|
||||
@@ -118,7 +102,7 @@ func TestStart_ConcurrentFirstCallersProduceOneChild(t *testing.T) {
|
||||
func TestStart_WillNotResurrectAnUnloadedPlugin(t *testing.T) {
|
||||
app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
|
||||
app.Use(must(zip.Load(
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: shortDir(t), Lazy: true}, "/v1/demo",
|
||||
zip.Plugin{Name: "demo", Bin: buildPlugin(t, "v1"), Dir: sockDir(t), Lazy: true}, "/v1/demo",
|
||||
)))
|
||||
t.Cleanup(func() { _ = app.Shutdown() })
|
||||
|
||||
|
||||
Reference in New Issue
Block a user