From 7672d60033ddc43f152103157295cdfd5747e255 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 17 Dec 2024 10:12:30 +0100 Subject: [PATCH 1/3] pkg/reexec: use const for name of test binary Also use a slightly different name, because "reexec" is used so widely as term in this package, making it somewhat confusing. Signed-off-by: Sebastiaan van Stijn --- pkg/reexec/reexec_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/reexec/reexec_test.go b/pkg/reexec/reexec_test.go index 5290c2adb8..0230cad585 100644 --- a/pkg/reexec/reexec_test.go +++ b/pkg/reexec/reexec_test.go @@ -6,8 +6,10 @@ import ( "testing" ) +const testReExec = "test-reexec" + func init() { - Register("reexec", func() { + Register(testReExec, func() { panic("Return Error") }) Init() @@ -16,17 +18,17 @@ func init() { func TestRegister(t *testing.T) { defer func() { if r := recover(); r != nil { - const expected = `reexec func already registered under name "reexec"` + const expected = `reexec func already registered under name "test-reexec"` if r != expected { t.Errorf("got %q, want %q", r, expected) } } }() - Register("reexec", func() {}) + Register(testReExec, func() {}) } func TestCommand(t *testing.T) { - cmd := Command("reexec") + cmd := Command(testReExec) w, err := cmd.StdinPipe() if err != nil { t.Fatalf("Error on pipe creation: %v", err) From 6568c06d12a638de9fc6a6208f7aea5b87534a9c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 17 Dec 2024 10:46:07 +0100 Subject: [PATCH 2/3] pkg/reexec: make platform-agnostic (again) The reexec package originally was platform-agnostic, but gained some Linux-specific handling in 1cb17f03d0b217acf2d2c289b4946d367f9d3e80. When Windows support was implemented in Docker, the pkg/reexec package was adjusted accordingly in 64715c4f33fdb98003b7fda8d1dcf9c65463de07, which now made the package with with either Linux or Windows, with various other platforms (freebsd, solaris, darwin) being added back in separate changes. Based on the history above, this package should be platform-agnostic, except for Linux-specific changes introduced in 1cb17f03d0b217acf2d2c289b4946d367f9d3e80 and 5aee8807a67687941916fc85c6d4da6bc59e834b. This patch: - removes the stub-implementation to make it functional on other platforms. - renames the files for consistency Signed-off-by: Sebastiaan van Stijn --- pkg/reexec/command_unsupported.go | 12 ------------ pkg/reexec/{command_linux.go => reexec_linux.go} | 0 pkg/reexec/{command_other.go => reexec_other.go} | 2 +- 3 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 pkg/reexec/command_unsupported.go rename pkg/reexec/{command_linux.go => reexec_linux.go} (100%) rename pkg/reexec/{command_other.go => reexec_other.go} (91%) diff --git a/pkg/reexec/command_unsupported.go b/pkg/reexec/command_unsupported.go deleted file mode 100644 index 3e98b989a3..0000000000 --- a/pkg/reexec/command_unsupported.go +++ /dev/null @@ -1,12 +0,0 @@ -//go:build !linux && !windows && !freebsd && !darwin - -package reexec - -import ( - "os/exec" -) - -// Command is unsupported on operating systems apart from Linux, Windows, and Darwin. -func Command(args ...string) *exec.Cmd { - return nil -} diff --git a/pkg/reexec/command_linux.go b/pkg/reexec/reexec_linux.go similarity index 100% rename from pkg/reexec/command_linux.go rename to pkg/reexec/reexec_linux.go diff --git a/pkg/reexec/command_other.go b/pkg/reexec/reexec_other.go similarity index 91% rename from pkg/reexec/command_other.go rename to pkg/reexec/reexec_other.go index b458ef2d20..369791baf0 100644 --- a/pkg/reexec/command_other.go +++ b/pkg/reexec/reexec_other.go @@ -1,4 +1,4 @@ -//go:build freebsd || darwin || windows +//go:build !linux package reexec From 8fd177d79b7db5f6e46beb78c57d31f62140646a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 17 Dec 2024 10:50:55 +0100 Subject: [PATCH 3/3] pkg/reexec: Command: separate public API from implementation Move the exported `Command` to a platform-agnostic file, and un-export the platform-specific implementations. This allows us to maintain the GoDoc in a single place, describing platform-specific differences where needed. Signed-off-by: Sebastiaan van Stijn --- pkg/reexec/reexec.go | 29 ++++++++++++++++++++++++----- pkg/reexec/reexec_linux.go | 12 +----------- pkg/reexec/reexec_other.go | 7 +------ 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/pkg/reexec/reexec.go b/pkg/reexec/reexec.go index b9d11a2a58..c3a0c925e9 100644 --- a/pkg/reexec/reexec.go +++ b/pkg/reexec/reexec.go @@ -3,7 +3,7 @@ // Handlers can be registered with a name and the argv 0 of the exec of // the binary will be used to find and execute custom init paths. // -// It is used in dockerd to work around forking limitations when using Go. +// It is used to work around forking limitations when using Go. package reexec import ( @@ -36,10 +36,29 @@ func Init() bool { return false } -// Self returns the path to the current process's binary. On Linux, it -// returns "/proc/self/exe", which provides the in-memory version of the -// current binary, whereas on other platforms it attempts to looks up the -// absolute path for os.Args[0], or otherwise returns os.Args[0] as-is. +// Command returns an [*exec.Cmd] with its Path set to the path of the current +// binary using the result of [Self]. +// +// On Linux, the Pdeathsig of [*exec.Cmd.SysProcAttr] is set to SIGTERM. +// This signal is sent to the process when the OS thread that created +// the process dies. +// +// It is the caller's responsibility to ensure that the creating thread is +// not terminated prematurely. See https://go.dev/issue/27505 for more details. +func Command(args ...string) *exec.Cmd { + return command(args...) +} + +// Self returns the path to the current process's binary. +// +// On Linux, it returns "/proc/self/exe", which provides the in-memory version +// of the current binary. This makes it safe to delete or replace the on-disk +// binary (os.Args[0]). +// +// On Other platforms, it attempts to look up the absolute path for os.Args[0], +// or otherwise returns os.Args[0] as-is. For example if current binary is +// "my-binary" at "/usr/bin/" (or "my-binary.exe" at "C:\" on Windows), +// then it returns "/usr/bin/my-binary" and "C:\my-binary.exe" respectively. func Self() string { if runtime.GOOS == "linux" { return "/proc/self/exe" diff --git a/pkg/reexec/reexec_linux.go b/pkg/reexec/reexec_linux.go index 952633c864..03f600e04f 100644 --- a/pkg/reexec/reexec_linux.go +++ b/pkg/reexec/reexec_linux.go @@ -5,17 +5,7 @@ import ( "syscall" ) -// Command returns an [*exec.Cmd] which has Path as current binary which, -// on Linux, is set to the in-memory version (/proc/self/exe) of the current -// binary, it is thus safe to delete or replace the on-disk binary (os.Args[0]). -// -// On Linux, the Pdeathsig of [*exec.Cmd.SysProcAttr] is set to SIGTERM. -// This signal will be sent to the process when the OS thread which created -// the process dies. -// -// It is the caller's responsibility to ensure that the creating thread is -// not terminated prematurely. See https://go.dev/issue/27505 for more details. -func Command(args ...string) *exec.Cmd { +func command(args ...string) *exec.Cmd { return &exec.Cmd{ Path: Self(), Args: args, diff --git a/pkg/reexec/reexec_other.go b/pkg/reexec/reexec_other.go index 369791baf0..498d28bc41 100644 --- a/pkg/reexec/reexec_other.go +++ b/pkg/reexec/reexec_other.go @@ -6,12 +6,7 @@ import ( "os/exec" ) -// Command returns *exec.Cmd with its Path set to the path of the current -// binary using the result of [Self]. For example if current binary is -// "my-binary" at "/usr/bin/" (or "my-binary.exe" at "C:\" on Windows), -// then cmd.Path is set to "/usr/bin/my-binary" and "C:\my-binary.exe" -// respectively. -func Command(args ...string) *exec.Cmd { +func command(args ...string) *exec.Cmd { return &exec.Cmd{ Path: Self(), Args: args,