Skip to content

Commit 258a4c3

Browse files
Richard Miller0intro
Richard Miller
authored andcommitted
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 <[email protected]> Run-TryBot: David du Colombier <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 2dc6800 commit 258a4c3

File tree

4 files changed

+1
-90
lines changed

4 files changed

+1
-90
lines changed

src/net/fd_plan9.go

-2
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,7 @@ func (l *TCPListener) dup() (*os.File, error) {
154154
}
155155

156156
func (fd *netFD) file(f *os.File, s string) (*os.File, error) {
157-
syscall.ForkLock.RLock()
158157
dfd, err := syscall.Dup(int(f.Fd()), -1)
159-
syscall.ForkLock.RUnlock()
160158
if err != nil {
161159
return nil, os.NewSyscallError("dup", err)
162160
}

src/net/file_plan9.go

-2
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ func newFileFD(f *os.File) (net *netFD, err error) {
5050
name := comp[2]
5151
switch file := comp[n-1]; file {
5252
case "ctl", "clone":
53-
syscall.ForkLock.RLock()
5453
fd, err := syscall.Dup(int(f.Fd()), -1)
55-
syscall.ForkLock.RUnlock()
5654
if err != nil {
5755
return nil, os.NewSyscallError("dup", err)
5856
}

src/os/file_plan9.go

-5
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,9 @@ func (file *file) close() error {
146146
return ErrInvalid
147147
}
148148
var err error
149-
syscall.ForkLock.RLock()
150149
if e := syscall.Close(file.fd); e != nil {
151150
err = &PathError{"close", file.name, e}
152151
}
153-
syscall.ForkLock.RUnlock()
154152
file.fd = -1 // so it can't be closed again
155153

156154
// no need for a finalizer anymore
@@ -420,12 +418,9 @@ func Chtimes(name string, atime time.Time, mtime time.Time) error {
420418
func Pipe() (r *File, w *File, err error) {
421419
var p [2]int
422420

423-
syscall.ForkLock.RLock()
424421
if e := syscall.Pipe(p[0:]); e != nil {
425-
syscall.ForkLock.RUnlock()
426422
return nil, nil, NewSyscallError("pipe", e)
427423
}
428-
syscall.ForkLock.RUnlock()
429424

430425
return NewFile(uintptr(p[0]), "|0"), NewFile(uintptr(p[1]), "|1"), nil
431426
}

src/syscall/exec_plan9.go

+1-81
Original file line numberDiff line numberDiff line change
@@ -12,53 +12,7 @@ import (
1212
"unsafe"
1313
)
1414

15-
// Lock synchronizing creation of new file descriptors with fork.
16-
//
17-
// We want the child in a fork/exec sequence to inherit only the
18-
// file descriptors we intend. To do that, we mark all file
19-
// descriptors close-on-exec and then, in the child, explicitly
20-
// unmark the ones we want the exec'ed program to keep.
21-
// Unix doesn't make this easy: there is, in general, no way to
22-
// allocate a new file descriptor close-on-exec. Instead you
23-
// have to allocate the descriptor and then mark it close-on-exec.
24-
// If a fork happens between those two events, the child's exec
25-
// will inherit an unwanted file descriptor.
26-
//
27-
// This lock solves that race: the create new fd/mark close-on-exec
28-
// operation is done holding ForkLock for reading, and the fork itself
29-
// is done holding ForkLock for writing. At least, that's the idea.
30-
// There are some complications.
31-
//
32-
// Some system calls that create new file descriptors can block
33-
// for arbitrarily long times: open on a hung NFS server or named
34-
// pipe, accept on a socket, and so on. We can't reasonably grab
35-
// the lock across those operations.
36-
//
37-
// It is worse to inherit some file descriptors than others.
38-
// If a non-malicious child accidentally inherits an open ordinary file,
39-
// that's not a big deal. On the other hand, if a long-lived child
40-
// accidentally inherits the write end of a pipe, then the reader
41-
// of that pipe will not see EOF until that child exits, potentially
42-
// causing the parent program to hang. This is a common problem
43-
// in threaded C programs that use popen.
44-
//
45-
// Luckily, the file descriptors that are most important not to
46-
// inherit are not the ones that can take an arbitrarily long time
47-
// to create: pipe returns instantly, and the net package uses
48-
// non-blocking I/O to accept on a listening socket.
49-
// The rules for which file descriptor-creating operations use the
50-
// ForkLock are as follows:
51-
//
52-
// 1) Pipe. Does not block. Use the ForkLock.
53-
// 2) Socket. Does not block. Use the ForkLock.
54-
// 3) Accept. If using non-blocking mode, use the ForkLock.
55-
// Otherwise, live with the race.
56-
// 4) Open. Can block. Use O_CLOEXEC if available (Linux).
57-
// Otherwise, live with the race.
58-
// 5) Dup. Does not block. Use the ForkLock.
59-
// On Linux, could use fcntl F_DUPFD_CLOEXEC
60-
// instead of the ForkLock, but only for dup(fd, -1).
61-
15+
// ForkLock is not used on plan9.
6216
var ForkLock sync.RWMutex
6317

6418
// 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) {
151105
return
152106
}
153107

154-
// readdupdevice returns a list of currently opened fds (excluding stdin, stdout, stderr) from the dup device #d.
155-
// ForkLock should be write locked before calling, so that no new fds would be created while the fd list is being read.
156-
func readdupdevice() (fds []int, err error) {
157-
dupdevfd, err := Open("#d", O_RDONLY)
158-
if err != nil {
159-
return
160-
}
161-
defer Close(dupdevfd)
162-
163-
names, err := readdirnames(dupdevfd)
164-
if err != nil {
165-
return
166-
}
167-
168-
fds = make([]int, 0, len(names)/2)
169-
for _, name := range names {
170-
if n := len(name); n > 3 && name[n-3:n] == "ctl" {
171-
continue
172-
}
173-
fd := int(atoi([]byte(name)))
174-
switch fd {
175-
case 0, 1, 2, dupdevfd:
176-
continue
177-
}
178-
fds = append(fds, fd)
179-
}
180-
return
181-
}
182-
183108
// name of the directory containing names and control files for all open file descriptors
184109
var dupdev, _ = BytePtrFromString("#d")
185110

@@ -492,9 +417,6 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
492417
}
493418
}
494419

495-
// Acquire the fork lock to prevent other threads from creating new fds before we fork.
496-
ForkLock.Lock()
497-
498420
// Allocate child status pipe close on exec.
499421
e := cexecPipe(p[:])
500422

@@ -510,10 +432,8 @@ func forkExec(argv0 string, argv []string, attr *ProcAttr) (pid int, err error)
510432
Close(p[0])
511433
Close(p[1])
512434
}
513-
ForkLock.Unlock()
514435
return 0, err
515436
}
516-
ForkLock.Unlock()
517437

518438
// Read child error status from pipe.
519439
Close(p[1])

0 commit comments

Comments
 (0)