From 509123f935e6fcb2cedf0df7bd8585a346082969 Mon Sep 17 00:00:00 2001 From: Bjorn Neergaard Date: Fri, 22 Mar 2024 08:55:18 -0600 Subject: [PATCH] plugin: drop explicit unlink Go's `net` package [will unlink][1] for us, as long as we used Listen & friends to create the Unix socket. Go will even skip the unlink when the socket appears to be abstract (starts with a NUL, represented by an @), though we must be cautious to only create sockets with an abstract address on platforms that actually support it -- this caused [several][2] [bugs][3] before. [1]: https://pkg.go.dev/net#UnixListener.SetUnlinkOnClose [2]: https://github.com/docker/cli/pull/4783 [3]: https://github.com/docker/cli/pull/4863 Signed-off-by: Bjorn Neergaard --- cli-plugins/socket/socket.go | 12 ++++++++---- cli-plugins/socket/socket_abstract.go | 19 ++++--------------- cli-plugins/socket/socket_noabstract.go | 19 ++++--------------- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/cli-plugins/socket/socket.go b/cli-plugins/socket/socket.go index 30c4afec05..fc91e78d8f 100644 --- a/cli-plugins/socket/socket.go +++ b/cli-plugins/socket/socket.go @@ -19,7 +19,14 @@ const EnvKey = "DOCKER_CLI_PLUGIN_SOCKET" // NewPluginServer creates a plugin server that listens on a new Unix domain // socket. h is called for each new connection to the socket in a goroutine. func NewPluginServer(h func(net.Conn)) (*PluginServer, error) { - l, err := listen("docker_cli_" + randomID()) + // Listen on a Unix socket, with the address being platform-dependent. + // When a non-abstract address is used, Go will unlink(2) the socket + // for us once the listener is closed, as documented in + // [net.UnixListener.SetUnlinkOnClose]. + l, err := net.ListenUnix("unix", &net.UnixAddr{ + Name: socketName("docker_cli_" + randomID()), + Net: "unix", + }) if err != nil { return nil, err } @@ -85,9 +92,6 @@ func (pl *PluginServer) Addr() net.Addr { // // The error value is that of the underlying [net.Listner.Close] call. func (pl *PluginServer) Close() error { - // Remove the listener socket, if it exists on the filesystem. - unlink(pl.l) - // Close connections first to ensure the connections get io.EOF instead // of a connection reset. pl.closeAllConns() diff --git a/cli-plugins/socket/socket_abstract.go b/cli-plugins/socket/socket_abstract.go index ce7b429036..a0276fa9c1 100644 --- a/cli-plugins/socket/socket_abstract.go +++ b/cli-plugins/socket/socket_abstract.go @@ -2,19 +2,8 @@ package socket -import ( - "net" -) - -func listen(socketname string) (*net.UnixListener, error) { - // Create an abstract socket -- this socket can be opened by name, but is - // not present in the filesystem. - return net.ListenUnix("unix", &net.UnixAddr{ - Name: "@" + socketname, - Net: "unix", - }) -} - -func unlink(listener *net.UnixListener) { - // Do nothing; the socket is not present in the filesystem. +func socketName(basename string) string { + // Address of an abstract socket -- this socket can be opened by name, + // but is not present in the filesystem. + return "@" + basename } diff --git a/cli-plugins/socket/socket_noabstract.go b/cli-plugins/socket/socket_noabstract.go index fbc948e6dc..7033a2a0c6 100644 --- a/cli-plugins/socket/socket_noabstract.go +++ b/cli-plugins/socket/socket_noabstract.go @@ -3,23 +3,12 @@ package socket import ( - "net" "os" "path/filepath" - "syscall" ) -func listen(socketname string) (*net.UnixListener, error) { - // Because abstract sockets are unavailable, we create a socket in the - // system temporary directory instead. - return net.ListenUnix("unix", &net.UnixAddr{ - Name: filepath.Join(os.TempDir(), socketname), - Net: "unix", - }) -} - -func unlink(listener *net.UnixListener) { - // unlink(2) is best effort here; if it fails, we may 'leak' a socket - // into the filesystem, but this is unlikely and overall harmless. - _ = syscall.Unlink(listener.Addr().String()) +func socketName(basename string) string { + // Because abstract sockets are unavailable, use a socket path in the + // system temporary directory. + return filepath.Join(os.TempDir(), basename) }