mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

os: fix a race between Process.signal() and wait() on Windows 

Process.handle was accessed without synchronization while wait() and
signal() could be called concurrently.

A first solution was to add a Mutex in Process but it was probably too
invasive given Process.handle is only used on Windows.

This version uses atomic operations to read the handle value. There is
still a race between isDone() and the value of the handle, but it only
leads to slightly incorrect error codes. The caller may get a:

  errors.New("os: process already finished")

instead of:

  syscall.EINVAL

which sounds harmless.

Fixes #9382

Change-Id: Iefcc687a1166d5961c8f27154647b9b15a0f748a
Reviewed-on: https://go-review.googlesource.com/9904
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit is contained in:
Patrick Mezard 2015-05-10 15:35:52 +02:00 committed by Alex Brainman
parent dc32b7f0fd
commit d574b59fc7
2 changed files with 13 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/os/exec.go
View File

@ -13,8 +13,8 @@ import (
// Process stores the information about a process created by StartProcess.
type Process struct {
Pid int
handle uintptr
isdone uint32 // process has been successfully waited on, non zero if true
handle uintptr // handle is accessed atomically on Windows
isdone uint32 // process has been successfully waited on, non zero if true
}
func newProcess(pid int, handle uintptr) *Process {

18
src/os/exec_windows.go
View File

@ -7,13 +7,15 @@ package os
import (
"errors"
"runtime"
"sync/atomic"
"syscall"
"time"
"unsafe"
)
func (p *Process) wait() (ps *ProcessState, err error) {
s, e := syscall.WaitForSingleObject(syscall.Handle(p.handle), syscall.INFINITE)
handle := atomic.LoadUintptr(&p.handle)
s, e := syscall.WaitForSingleObject(syscall.Handle(handle), syscall.INFINITE)
switch s {
case syscall.WAIT_OBJECT_0:
break
@ -23,12 +25,12 @@ func (p *Process) wait() (ps *ProcessState, err error) {
return nil, errors.New("os: unexpected result from WaitForSingleObject")
}
var ec uint32
e = syscall.GetExitCodeProcess(syscall.Handle(p.handle), &ec)
e = syscall.GetExitCodeProcess(syscall.Handle(handle), &ec)
if e != nil {
return nil, NewSyscallError("GetExitCodeProcess", e)
}
var u syscall.Rusage
e = syscall.GetProcessTimes(syscall.Handle(p.handle), &u.CreationTime, &u.ExitTime, &u.KernelTime, &u.UserTime)
e = syscall.GetProcessTimes(syscall.Handle(handle), &u.CreationTime, &u.ExitTime, &u.KernelTime, &u.UserTime)
if e != nil {
return nil, NewSyscallError("GetProcessTimes", e)
}
@ -53,7 +55,8 @@ func terminateProcess(pid, exitcode int) error {
}
func (p *Process) signal(sig Signal) error {
if p.handle == uintptr(syscall.InvalidHandle) {
handle := atomic.LoadUintptr(&p.handle)
if handle == uintptr(syscall.InvalidHandle) {
return syscall.EINVAL
}
if p.done() {
@ -67,14 +70,15 @@ func (p *Process) signal(sig Signal) error {
}
func (p *Process) release() error {
if p.handle == uintptr(syscall.InvalidHandle) {
handle := atomic.LoadUintptr(&p.handle)
if handle == uintptr(syscall.InvalidHandle) {
return syscall.EINVAL
}
e := syscall.CloseHandle(syscall.Handle(p.handle))
e := syscall.CloseHandle(syscall.Handle(handle))
if e != nil {
return NewSyscallError("CloseHandle", e)
}
p.handle = uintptr(syscall.InvalidHandle)
atomic.StoreUintptr(&p.handle, uintptr(syscall.InvalidHandle))
// no need for a finalizer anymore
runtime.SetFinalizer(p, nil)
return nil