From 258a4c3daf992958f5d7dc5bccf2c5b41e236959 Mon Sep 17 00:00:00 2001 From: Richard Miller Date: Fri, 6 May 2016 14:21:52 +0100 Subject: [PATCH] syscall,os,net: don't use ForkLock in plan9 This is the follow-on to CL 22610: now that it's the child instead of the parent which lists unwanted fds to close in syscall.StartProcess, plan9 no longer needs the ForkLock to protect the list from changing. The readdupdevice function is also now unused and can be removed. Change-Id: I904c8bbf5dbaa7022b0f1a1de0862cd3064ca8c7 Reviewed-on: https://go-review.googlesource.com/22842 Reviewed-by: David du Colombier <0intro@gmail.com> Run-TryBot: David du Colombier <0intro@gmail.com> Reviewed-by: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/fd_plan9.go | 2 - src/net/file_plan9.go | 2 - src/os/file_plan9.go | 5 --- src/syscall/exec_plan9.go | 82 +-------------------------------------- 4 files changed, 1 insertion(+), 90 deletions(-) diff --git a/src/net/fd_plan9.go b/src/net/fd_plan9.go index 329d6152b2..8e272b1eb8 100644 --- a/src/net/fd_plan9.go +++ b/src/net/fd_plan9.go @@ -154,9 +154,7 @@ func (l *TCPListener) dup() (*os.File, error) { } func (fd *netFD) file(f *os.File, s string) (*os.File, error) { - syscall.ForkLock.RLock() dfd, err := syscall.Dup(int(f.Fd()), -1) - syscall.ForkLock.RUnlock() if err != nil { return nil, os.NewSyscallError("dup", err) } diff --git a/src/net/file_plan9.go b/src/net/file_plan9.go index 24efdc5186..2939c09a43 100644 --- a/src/net/file_plan9.go +++ b/src/net/file_plan9.go @@ -50,9 +50,7 @@ func newFileFD(f *os.File) (net *netFD, err error) { name := comp[2] switch file := comp[n-1]; file { case "ctl", "clone": - syscall.ForkLock.RLock() fd, err := syscall.Dup(int(f.Fd()), -1) - syscall.ForkLock.RUnlock() if err != nil { return nil, os.NewSyscallError("dup", err) } diff --git a/src/os/file_plan9.go b/src/os/file_plan9.go index fb796a2a89..9edb6bc074 100644 --- a/src/os/file_plan9.go +++ b/src/os/file_plan9.go @@ -146,11 +146,9 @@ func (file *file) close() error { return ErrInvalid } var err error - syscall.ForkLock.RLock() if e := syscall.Close(file.fd); e != nil { err = &PathError{"close", file.name, e} } - syscall.ForkLock.RUnlock() file.fd = -1 // so it can't be closed again // no need for a finalizer anymore @@ -420,12 +418,9 @@ func Chtimes(name string, atime time.Time, mtime time.Time) error { func Pipe() (r *File, w *File, err error) { var p [2]int - syscall.ForkLock.RLock() if e := syscall.Pipe(p[0:]); e != nil { - syscall.ForkLock.RUnlock() return nil, nil, NewSyscallError("pipe", e) } - syscall.ForkLock.RUnlock() return NewFile(uintptr(p[0]), "|0"), NewFile(uintptr(p[1]), "|1"), nil } diff --git a/src/syscall/exec_plan9.go b/src/syscall/exec_plan9.go index 58e5a3c623..6551bcb1c1 100644 --- a/src/syscall/exec_plan9.go +++ b/src/syscall/exec_plan9.go @@ -12,53 +12,7 @@ import ( "unsafe" ) -// Lock synchronizing creation of new file descriptors with fork. -// -// We want the child in a fork/exec sequence to inherit only the -// file descriptors we intend. To do that, we mark all file -// descriptors close-on-exec and then, in the child, explicitly -// unmark the ones we want the exec'ed program to keep. -// Unix doesn't make this easy: there is, in general, no way to -// allocate a new file descriptor close-on-exec. Instead you -// have to allocate the descriptor and then mark it close-on-exec. -// If a fork happens between those two events, the child's exec -// will inherit an unwanted file descriptor. -// -// This lock solves that race: the create new fd/mark close-on-exec -// operation is done holding ForkLock for reading, and the fork itself -// is done holding ForkLock for writing. At least, that's the idea. -// There are some complications. -// -// Some system calls that create new file descriptors can block -// for arbitrarily long times: open on a hung NFS server or named -// pipe, accept on a socket, and so on. We can't reasonably grab -// the lock across those operations. -// -// It is worse to inherit some file descriptors than others. -// If a non-malicious child accidentally inherits an open ordinary file, -// that's not a big deal. On the other hand, if a long-lived child -// accidentally inherits the write end of a pipe, then the reader -// of that pipe will not see EOF until that child exits, potentially -// causing the parent program to hang. This is a common problem -// in threaded C programs that use popen. -// -// Luckily, the file descriptors that are most important not to -// inherit are not the ones that can take an arbitrarily long time -// to create: pipe returns instantly, and the net package uses -// non-blocking I/O to accept on a listening socket. -// The rules for which file descriptor-creating operations use the -// ForkLock are as follows: -// -// 1) Pipe. Does not block. Use the ForkLock. -// 2) Socket. Does not block. Use the ForkLock. -// 3) Accept. If using non-blocking mode, use the ForkLock. -// Otherwise, live with the race. -// 4) Open. Can block. Use O_CLOEXEC if available (Linux). -// Otherwise, live with the race. -// 5) Dup. Does not block. Use the ForkLock. -// On Linux, could use fcntl F_DUPFD_CLOEXEC -// instead of the ForkLock, but only for dup(fd, -1). - +// ForkLock is not used on plan9. var ForkLock sync.RWMutex // gstringb reads a non-empty string from b, prefixed with a 16-bit length in little-endian order. @@ -151,35 +105,6 @@ func readdirnames(dirfd int) (names []string, err error) { return } -// readdupdevice returns a list of currently opened fds (excluding stdin, stdout, stderr) from the dup device #d. -// ForkLock should be write locked before calling, so that no new fds would be created while the fd list is being read. -func readdupdevice() (fds []int, err error) { - dupdevfd, err := Open("#d", O_RDONLY) - if err != nil { - return - } - defer Close(dupdevfd) - - names, err := readdirnames(dupdevfd) - if err != nil { - return - } - - fds = make([]int, 0, len(names)/2) - for _, name := range names { - if n := len(name); n > 3 && name[n-3:n] == "ctl" { - continue - } - fd := int(atoi([]byte(name))) - switch fd { - case 0, 1, 2, dupdevfd: - continue - } - fds = append(fds, fd) - } - return -} - // name of the directory containing names and control files for all open file descriptors var dupdev, _ = BytePtrFromString("#d") @@ -492,9 +417,6 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error) } } - // Acquire the fork lock to prevent other threads from creating new fds before we fork. - ForkLock.Lock() - // Allocate child status pipe close on exec. e := cexecPipe(p[:]) @@ -510,10 +432,8 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error) Close(p[0]) Close(p[1]) } - ForkLock.Unlock() return 0, err } - ForkLock.Unlock() // Read child error status from pipe. Close(p[1]) -- 2.50.0