Commit 258a4c3d authored by Richard Miller's avatar Richard Miller Committed by David du Colombier

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/22842Reviewed-by: 's avatarDavid du Colombier <0intro@gmail.com>
Run-TryBot: David du Colombier <0intro@gmail.com>
Reviewed-by: 's avatarBrad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 2dc68000
......@@ -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)
}
......
......@@ -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)
}
......
......@@ -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
}
......
......@@ -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])
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment