]> Cypherpunks repositories - gostls13.git/commitdiff
syscall,os,net: don't use ForkLock in plan9
authorRichard Miller <miller.research@gmail.com>
Fri, 6 May 2016 13:21:52 +0000 (14:21 +0100)
committerDavid du Colombier <0intro@gmail.com>
Fri, 6 May 2016 16:43:07 +0000 (16:43 +0000)
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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/net/fd_plan9.go
src/net/file_plan9.go
src/os/file_plan9.go
src/syscall/exec_plan9.go

index 329d6152b2d9894a2e392c16b14c27bc6d4bb80e..8e272b1eb85e53b3cfc2abc8a14a5c465133a841 100644 (file)
@@ -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)
        }
index 24efdc5186d89f9d41d04bbe5ec56d9266b1279a..2939c09a43097f94503ba2a5204e195eb5e82d53 100644 (file)
@@ -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)
                }
index fb796a2a89bcbaa66281308597f3339448d36c5a..9edb6bc0747d08fbacd3aded37af37fa1f6c1a81 100644 (file)
@@ -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
 }
index 58e5a3c623b62aaa4aa9ee50c20654abe9ea794e..6551bcb1c1f21570d767751fe040a51eba44b646 100644 (file)
@@ -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])