New Upstream Snapshot - golang-github-cloudflare-tableflip
Ready changes
Summary
Merged new upstream version: 1.2.3 (was: 1.2.1~git20200514.4baec98).
Resulting package
Built on 2022-11-26T17:47 (took 16m23s)
The resulting binary packages can be installed (if you have the apt repository enabled) by running one of:
apt install -t fresh-snapshots golang-github-cloudflare-tableflip-dev
Lintian Result
Diff
diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index c99c8c8..0000000
--- a/.travis.yml
+++ /dev/null
@@ -1,10 +0,0 @@
-language: go
-
-go:
- - "1.13.x"
- - "1.14.x"
-
-os:
- - linux
- - osx
- - windows
diff --git a/README.md b/README.md
index ae7abff..4c1433f 100644
--- a/README.md
+++ b/README.md
@@ -49,7 +49,7 @@ Please see the more elaborate [graceful shutdown with net/http](http_example_tes
## Integration with `systemd`
-```
+```text
[Unit]
Description=Service using tableflip
@@ -61,4 +61,9 @@ PIDFile=/path/to/pid-file
See the [documentation](https://godoc.org/github.com/cloudflare/tableflip) as well.
-The logs of a process using `tableflip` may go missing due to a [bug in journald](https://github.com/systemd/systemd/issues/13708). You can work around this by logging directly to journald, for example by using [go-systemd/journal](https://godoc.org/github.com/coreos/go-systemd/journal) and looking for the [$JOURNAL_STREAM](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#$JOURNAL_STREAM) environment variable.
+The logs of a process using `tableflip` may go missing due to a [bug in journald](https://github.com/systemd/systemd/issues/13708),
+which has been fixed by systemd v244 release. If you are running an older version
+of systemd, you can work around this by logging directly to journald, for example
+by using [go-systemd/journal](https://godoc.org/github.com/coreos/go-systemd/journal)
+and looking for the [$JOURNAL_STREAM](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#$JOURNAL_STREAM)
+environment variable.
diff --git a/child.go b/child.go
index bc4164f..1b5e04d 100644
--- a/child.go
+++ b/child.go
@@ -31,7 +31,7 @@ func startChild(env *env, passedFiles map[fileName]*file) (*child, error) {
}
// Copy passed fds and append the notification pipe
- fds := []*os.File{readyW, namesR}
+ fds := []*os.File{os.Stdin, os.Stdout, os.Stderr, readyW, namesR}
var fdNames [][]string
for name, file := range passedFiles {
nameSlice := make([]string, len(name))
@@ -41,9 +41,14 @@ func startChild(env *env, passedFiles map[fileName]*file) (*child, error) {
}
// Copy environment and append the notification env vars
- environ := append([]string(nil), env.environ()...)
- environ = append(environ,
- fmt.Sprintf("%s=yes", sentinelEnvVar))
+ sentinel := fmt.Sprintf("%s=yes", sentinelEnvVar)
+ var environ []string
+ for _, val := range env.environ() {
+ if val != sentinel {
+ environ = append(environ, val)
+ }
+ }
+ environ = append(environ, sentinel)
proc, err := env.newProc(os.Args[0], os.Args[1:], fds, environ)
if err != nil {
diff --git a/child_test.go b/child_test.go
index dc15c73..bd32804 100644
--- a/child_test.go
+++ b/child_test.go
@@ -83,8 +83,8 @@ func TestChildPassedFds(t *testing.T) {
}
in := map[fileName]*file{
- fileName{"r"}: newFile(r.Fd(), fileName{"r"}),
- fileName{"w"}: newFile(w.Fd(), fileName{"w"}),
+ {"r"}: newFile(r.Fd(), fileName{"r"}),
+ {"w"}: newFile(w.Fd(), fileName{"w"}),
}
if _, err := startChild(env, in); err != nil {
@@ -92,15 +92,15 @@ func TestChildPassedFds(t *testing.T) {
}
proc := <-procs
- if len(proc.fds) != 2+2 {
- t.Error("Expected 4 files, got", len(proc.fds))
- }
-
out, _, err := proc.notify()
if err != nil {
t.Fatal("Notify failed:", err)
}
+ if len(out) != len(in) {
+ t.Errorf("Expected %d files, got %d", len(in), len(out))
+ }
+
for name, inFd := range in {
if outFd, ok := out[name]; !ok {
t.Error(name, "is missing")
diff --git a/debian/changelog b/debian/changelog
index 353b4d3..1a96d43 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+golang-github-cloudflare-tableflip (1.2.3-1) UNRELEASED; urgency=low
+
+ * New upstream release.
+
+ -- Debian Janitor <janitor@jelmer.uk> Sat, 26 Nov 2022 17:36:47 -0000
+
golang-github-cloudflare-tableflip (1.2.1~git20200514.4baec98-4) unstable; urgency=medium
[ Debian Janitor ]
diff --git a/dup_fd.go b/dup_fd.go
index 5741269..9f7a672 100644
--- a/dup_fd.go
+++ b/dup_fd.go
@@ -1,3 +1,4 @@
+//go:build !windows
// +build !windows
package tableflip
diff --git a/dup_file.go b/dup_file.go
deleted file mode 100644
index 43a52c4..0000000
--- a/dup_file.go
+++ /dev/null
@@ -1,12 +0,0 @@
-// +build go1.12
-
-package tableflip
-
-import (
- "os"
-)
-
-func dupFile(fh *os.File, name fileName) (*file, error) {
- // os.File implements syscall.Conn from go 1.12
- return dupConn(fh, name)
-}
diff --git a/dup_file_legacy.go b/dup_file_legacy.go
deleted file mode 100644
index b045837..0000000
--- a/dup_file_legacy.go
+++ /dev/null
@@ -1,11 +0,0 @@
-// +build !go1.12
-
-package tableflip
-
-import (
- "os"
-)
-
-func dupFile(fh *os.File, name fileName) (*file, error) {
- return dupFd(fh.Fd(), name)
-}
diff --git a/env_syscalls.go b/env_syscalls.go
index 7cefac8..a5d395f 100644
--- a/env_syscalls.go
+++ b/env_syscalls.go
@@ -1,3 +1,4 @@
+//go:build !windows
// +build !windows
package tableflip
diff --git a/fds.go b/fds.go
index 4967b84..a0f3a77 100644
--- a/fds.go
+++ b/fds.go
@@ -75,8 +75,7 @@ func newFile(fd uintptr, name fileName) *file {
// Fds holds all file descriptors inherited from the
// parent process.
type Fds struct {
- mu sync.Mutex
- // NB: Files in these maps may be in blocking mode.
+ mu sync.Mutex
inherited map[fileName]*file
used map[fileName]*file
lc *net.ListenConfig
@@ -98,8 +97,22 @@ func newFds(inherited map[fileName]*file, lc *net.ListenConfig) *Fds {
}
}
+func (f *Fds) newListener(network, addr string) (net.Listener, error) {
+ return f.lc.Listen(context.Background(), network, addr)
+}
+
// Listen returns a listener inherited from the parent process, or creates a new one.
func (f *Fds) Listen(network, addr string) (net.Listener, error) {
+ return f.ListenWithCallback(network, addr, f.newListener)
+}
+
+// ListenWithCallback returns a listener inherited from the parent process,
+// or calls the supplied callback to create a new one.
+//
+// This should be used in case some customization has to be applied to create the
+// connection. Note that the callback must not use the underlying `Fds` object
+// as it will be locked during the call.
+func (f *Fds) ListenWithCallback(network, addr string, callback func(network, addr string) (net.Listener, error)) (net.Listener, error) {
f.mu.Lock()
defer f.mu.Unlock()
@@ -112,7 +125,7 @@ func (f *Fds) Listen(network, addr string) (net.Listener, error) {
return ln, nil
}
- ln, err = f.lc.Listen(context.Background(), network, addr)
+ ln, err = callback(network, addr)
if err != nil {
return nil, fmt.Errorf("can't create new listener: %s", err)
}
@@ -181,8 +194,22 @@ func (f *Fds) addListenerLocked(network, addr string, ln Listener) error {
return f.addSyscallConnLocked(listenKind, network, addr, ln)
}
+func (f *Fds) newPacketConn(network, addr string) (net.PacketConn, error) {
+ return f.lc.ListenPacket(context.Background(), network, addr)
+}
+
// ListenPacket returns a packet conn inherited from the parent process, or creates a new one.
func (f *Fds) ListenPacket(network, addr string) (net.PacketConn, error) {
+ return f.ListenPacketWithCallback(network, addr, f.newPacketConn)
+}
+
+// ListenPacketWithCallback returns a packet conn inherited from the parent process,
+// or calls the supplied callback to create a new one.
+//
+// This should be used in case some customization has to be applied to create the
+// connection. Note that the callback must not use the underlying `Fds` object
+// as it will be locked during the call.
+func (f *Fds) ListenPacketWithCallback(network, addr string, callback func(network, addr string) (net.PacketConn, error)) (net.PacketConn, error) {
f.mu.Lock()
defer f.mu.Unlock()
@@ -195,7 +222,7 @@ func (f *Fds) ListenPacket(network, addr string) (net.PacketConn, error) {
return conn, nil
}
- conn, err = f.lc.ListenPacket(context.Background(), network, addr)
+ conn, err = callback(network, addr)
if err != nil {
return nil, fmt.Errorf("can't create new listener: %s", err)
}
@@ -296,6 +323,36 @@ func (f *Fds) addSyscallConnLocked(kind, network, addr string, conn syscall.Conn
return nil
}
+// Files returns all inherited files and mark them as used.
+//
+// The descriptors may be in blocking mode.
+func (f *Fds) Files() ([]*os.File, error) {
+ f.mu.Lock()
+ defer f.mu.Unlock()
+
+ var files []*os.File
+
+ for key, file := range f.inherited {
+ if key[0] != fdKind {
+ continue
+ }
+
+ // Make a copy of the file, since we don't want to
+ // allow the caller to invalidate fds in f.inherited.
+ dup, err := dupFd(file.fd, key)
+ if err != nil {
+ return nil, err
+ }
+
+ f.used[key] = file
+ delete(f.inherited, key)
+
+ files = append(files, dup.File)
+ }
+
+ return files, nil
+}
+
// File returns an inherited file or nil.
//
// The descriptor may be in blocking mode.
@@ -322,13 +379,10 @@ func (f *Fds) File(name string) (*os.File, error) {
}
// AddFile adds a file.
-//
-// Until Go 1.12, file will be in blocking mode
-// after this call.
func (f *Fds) AddFile(name string, file *os.File) error {
key := fileName{fdKind, name}
- dup, err := dupFile(file, key)
+ dup, err := dupConn(file, key)
if err != nil {
return err
}
@@ -430,3 +484,22 @@ func dupConn(conn syscall.Conn, name fileName) (*file, error) {
}
return dup, duperr
}
+
+// sysConnFd retrieves the fd for a syscall.Conn.
+//
+// Don't close the conn while using the fd.
+func sysConnFd(conn syscall.Conn) (uintptr, error) {
+ raw, err := conn.SyscallConn()
+ if err != nil {
+ return 0, err
+ }
+
+ var fd uintptr
+ err = raw.Control(func(fdArg uintptr) {
+ fd = fdArg
+ })
+ if err != nil {
+ return 0, fmt.Errorf("can't access fd: %s", err)
+ }
+ return fd, nil
+}
diff --git a/fds_test.go b/fds_test.go
index 07b9c4c..b066022 100644
--- a/fds_test.go
+++ b/fds_test.go
@@ -127,6 +127,75 @@ func TestFdsListen(t *testing.T) {
}
}
+func TestFdsListenWithCallback(t *testing.T) {
+ socketPath, cleanup := tempSocket(t)
+ defer cleanup()
+
+ addrs := [][2]string{
+ {"tcp", "localhost:0"},
+ {"udp", "localhost:0"},
+ {"unix", socketPath},
+ {"unixgram", socketPath + "Unixgram"},
+ }
+ parent := newFds(nil, nil)
+
+ var (
+ ln io.Closer
+ err error
+ )
+
+ called := false
+ packetCb := func(network, addr string) (net.PacketConn, error) {
+ called = true
+ return net.ListenPacket(network, addr)
+ }
+ listenerCb := func(network, addr string) (net.Listener, error) {
+ called = true
+ return net.Listen(network, addr)
+ }
+
+ for _, addr := range addrs {
+ called = false
+ switch addr[0] {
+ case "udp", "unixgram":
+ ln, err = parent.ListenPacketWithCallback(addr[0], addr[1], packetCb)
+ default:
+ ln, err = parent.ListenWithCallback(addr[0], addr[1], listenerCb)
+ }
+ if err != nil {
+ t.Fatalf("Can't create %s listener: %s", addr[0], err)
+ }
+ if ln == nil {
+ t.Fatalf("Got a nil %s listener", addr[0])
+ }
+ if !called {
+ t.Fatalf("Callback not called for new %s listener", addr[0])
+ }
+ ln.Close()
+ }
+
+ child := newFds(parent.copy(), nil)
+ for _, addr := range addrs {
+ called = false
+ switch addr[0] {
+ case "udp", "unixgram":
+ ln, err = child.ListenPacketWithCallback(addr[0], addr[1], packetCb)
+ default:
+ ln, err = child.ListenWithCallback(addr[0], addr[1], listenerCb)
+ }
+ if err != nil {
+ t.Fatalf("Can't get retrieve %s from child: %s", addr[0], err)
+ }
+ if ln == nil {
+ t.Fatalf("Missing %s listener", addr[0])
+ }
+ if called {
+ t.Fatalf("Callback called for inherited %s listener", addr[0])
+ }
+ ln.Close()
+ }
+}
+
func TestFdsRemoveUnix(t *testing.T) {
socketPath, cleanup := tempSocket(t)
defer cleanup()
@@ -246,3 +315,62 @@ func TestFdsFile(t *testing.T) {
}
file.Close()
}
+
+func TestFdsFiles(t *testing.T) {
+ r1, w1, err := os.Pipe()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer r1.Close()
+
+ r2, w2, err := os.Pipe()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer r2.Close()
+
+ testcases := []struct {
+ f *os.File
+ name string
+ expected string
+ }{
+ {
+ w1,
+ "test1",
+ "fd:test1:",
+ },
+ {
+ w2,
+ "test2",
+ "fd:test2:",
+ },
+ }
+
+ parent := newFds(nil, nil)
+ for _, tc := range testcases {
+ if err := parent.AddFile(tc.name, tc.f); err != nil {
+ t.Fatal("Can't add file:", err)
+ }
+ tc.f.Close()
+ }
+
+ child := newFds(parent.copy(), nil)
+ files, err := child.Files()
+ if err != nil {
+ t.Fatal("Can't get inherited files:", err)
+ }
+
+ if len(files) != len(testcases) {
+ t.Fatalf("Expected %d files, got %d", len(testcases), len(files))
+ }
+
+ for i, ff := range files {
+ tc := testcases[i]
+
+ if ff.Name() != tc.expected {
+ t.Errorf("Expected file %q, got %q", tc.expected, ff.Name())
+ }
+
+ ff.Close()
+ }
+}
diff --git a/go.mod b/go.mod
index 04fb41c..7c90929 100644
--- a/go.mod
+++ b/go.mod
@@ -1,3 +1,5 @@
module github.com/cloudflare/tableflip
-go 1.13
+go 1.14
+
+require golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4
diff --git a/go.sum b/go.sum
index e69de29..9b4cc1b 100644
--- a/go.sum
+++ b/go.sum
@@ -0,0 +1,2 @@
+golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=
+golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
diff --git a/process.go b/process.go
index c918a88..8b7dc8b 100644
--- a/process.go
+++ b/process.go
@@ -4,6 +4,8 @@ import (
"fmt"
"os"
"os/exec"
+ "runtime"
+ "syscall"
)
var initialWD, _ = os.Getwd()
@@ -15,33 +17,66 @@ type process interface {
}
type osProcess struct {
- cmd *exec.Cmd
+ *os.Process
+ finished bool
}
func newOSProcess(executable string, args []string, files []*os.File, env []string) (process, error) {
- cmd := exec.Command(executable, args...)
- cmd.Dir = initialWD
- cmd.Stdin = os.Stdin
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
- cmd.ExtraFiles = files
- cmd.Env = env
-
- if err := cmd.Start(); err != nil {
+ executable, err := exec.LookPath(executable)
+ if err != nil {
return nil, err
}
- return &osProcess{cmd}, nil
-}
+ fds := make([]uintptr, 0, len(files))
+ for _, file := range files {
+ fd, err := sysConnFd(file)
+ if err != nil {
+ return nil, err
+ }
+ fds = append(fds, fd)
+ }
+
+ attr := &syscall.ProcAttr{
+ Dir: initialWD,
+ Env: env,
+ Files: fds,
+ }
-func (osp *osProcess) Signal(sig os.Signal) error {
- return osp.cmd.Process.Signal(sig)
+ args = append([]string{executable}, args...)
+ pid, _, err := syscall.StartProcess(executable, args, attr)
+ if err != nil {
+ return nil, fmt.Errorf("fork/exec: %s", err)
+ }
+
+ // Ensure that fds stay valid until after StartProcess finishes.
+ runtime.KeepAlive(files)
+
+ proc, err := os.FindProcess(pid)
+ if err != nil {
+ return nil, fmt.Errorf("find pid %d: %s", pid, err)
+ }
+
+ return &osProcess{Process: proc}, nil
}
func (osp *osProcess) Wait() error {
- return osp.cmd.Wait()
+ if osp.finished {
+ return fmt.Errorf("already waited")
+ }
+ osp.finished = true
+
+ state, err := osp.Process.Wait()
+ if err != nil {
+ return err
+ }
+
+ if !state.Success() {
+ return &exec.ExitError{ProcessState: state}
+ }
+
+ return nil
}
func (osp *osProcess) String() string {
- return fmt.Sprintf("pid=%d", osp.cmd.Process.Pid)
+ return fmt.Sprintf("pid=%d", osp.Pid)
}
diff --git a/process_test.go b/process_test.go
index 7b44170..105f9b4 100644
--- a/process_test.go
+++ b/process_test.go
@@ -1,11 +1,95 @@
package tableflip
import (
+ "errors"
"fmt"
"os"
+ "os/exec"
"strings"
+ "testing"
+
+ "golang.org/x/sys/unix"
)
+func TestFilesAreNonblocking(t *testing.T) {
+ pipe := func() (r, w *os.File) {
+ r, w, err := os.Pipe()
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() {
+ r.Close()
+ w.Close()
+ })
+ return r, w
+ }
+
+ // Set up our own blocking stdin since CI runs tests with stdin closed.
+ rStdin, _ := pipe()
+ rStdin.Fd()
+
+ r, _ := pipe()
+ if !isNonblock(t, r) {
+ t.Fatal("Read pipe is blocking")
+ }
+
+ proc, err := newOSProcess("cat", nil, []*os.File{rStdin, os.Stdout, os.Stderr, r}, nil)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ if err := proc.Signal(os.Kill); err != nil {
+ t.Fatal("Can't signal:", err)
+ }
+
+ var exitErr *exec.ExitError
+ if err := proc.Wait(); !errors.As(err, &exitErr) {
+ t.Fatalf("Wait should return an ExitError after sending os.Kill, have %T: %s", err, err)
+ }
+
+ if err := proc.Wait(); err == nil {
+ t.Fatal("Waiting a second time should return an error")
+ }
+
+ if !isNonblock(t, r) {
+ t.Fatal("Read pipe is blocking after newOSProcess")
+ }
+}
+
+func TestArgumentsArePassedCorrectly(t *testing.T) {
+ proc, err := newOSProcess("printf", []string{""}, []*os.File{os.Stdin, os.Stdout, os.Stderr}, nil)
+ if err != nil {
+ t.Fatal("Can't execute printf:", err)
+ }
+
+ // If the argument handling is wrong we'll call printf without any arguments.
+ // In that case printf exits non-zero.
+ if err = proc.Wait(); err != nil {
+ t.Fatal("printf exited non-zero:", err)
+ }
+}
+
+func isNonblock(tb testing.TB, file *os.File) (nonblocking bool) {
+ tb.Helper()
+
+ raw, err := file.SyscallConn()
+ if err != nil {
+ tb.Fatal("SyscallConn:", err)
+ }
+
+ err = raw.Control(func(fd uintptr) {
+ flags, err := unix.FcntlInt(fd, unix.F_GETFL, 0)
+ if err != nil {
+ tb.Fatal("IsNonblock:", err)
+ }
+ nonblocking = flags&unix.O_NONBLOCK > 0
+ })
+ if err != nil {
+ tb.Fatal("Control:", err)
+ }
+ return
+}
+
type testProcess struct {
fds []*os.File
env env
@@ -29,7 +113,7 @@ func newTestProcess(fds []*os.File, envstr []string) (*testProcess, error) {
fds,
env{
newFile: func(fd uintptr, name string) *os.File {
- return fds[fd-3]
+ return fds[fd]
},
getenv: func(key string) string {
return environ[key]
Debdiff
[The following lists of changes regard files as different if they have different names, permissions or owners.]
Files in first set of .debs but not in second
-rw-r--r-- root/root /usr/share/gocode/src/github.com/cloudflare/tableflip/dup_file.go -rw-r--r-- root/root /usr/share/gocode/src/github.com/cloudflare/tableflip/dup_file_legacy.go
No differences were encountered in the control files