Skip to content

Commit

Permalink
syscall: use runtime.KeepAlive for ProcThreadAttributeList arguments
Browse files Browse the repository at this point in the history
It turns out that if you write Go pointers to Go memory, the Go compiler
must be involved so that it generates various calls to the GC in the
process. Letting Windows write Go pointers to Go memory violated this.
So, we replace that with just a boring call to runtime.KeepAlive. That's
not a great API, but this is all internal code anyway. We fix it up
more elegantly for external consumption in x/sys/windows with CL 300369.

Fixes #44900.

Change-Id: Id6599a793af9c4815f6c9387b00796923f32cb97
Reviewed-on: https://go-review.googlesource.com/c/go/+/300349
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
  • Loading branch information
zx2c4 committed Mar 11, 2021
1 parent 9ece63f commit b8e9ec8
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 44 deletions.
3 changes: 3 additions & 0 deletions src/syscall/exec_windows.go
Expand Up @@ -7,6 +7,7 @@
package syscall

import (
"runtime"
"sync"
"unicode/utf16"
"unsafe"
Expand Down Expand Up @@ -368,6 +369,8 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
return 0, 0, err
}
defer CloseHandle(Handle(pi.Thread))
runtime.KeepAlive(fd)
runtime.KeepAlive(sys)

return int(pi.ProcessId), uintptr(pi.Process), nil
}
Expand Down
2 changes: 1 addition & 1 deletion src/syscall/syscall_windows.go
Expand Up @@ -1257,7 +1257,7 @@ func newProcThreadAttributeList(maxAttrCount uint32) (*_PROC_THREAD_ATTRIBUTE_LI
return nil, err
}
// size is guaranteed to be ≥1 by initializeProcThreadAttributeList.
al := (*_PROC_THREAD_ATTRIBUTE_LIST)(unsafe.Pointer(&make([]unsafe.Pointer, (size+ptrSize-1)/ptrSize)[0]))
al := (*_PROC_THREAD_ATTRIBUTE_LIST)(unsafe.Pointer(&make([]byte, size)[0]))
err = initializeProcThreadAttributeList(al, maxAttrCount, 0, &size)
if err != nil {
return nil, err
Expand Down
36 changes: 0 additions & 36 deletions src/syscall/syscall_windows_test.go
Expand Up @@ -7,11 +7,8 @@ package syscall_test
import (
"os"
"path/filepath"
"runtime"
"syscall"
"testing"
"time"
"unsafe"
)

func TestWin32finddata(t *testing.T) {
Expand Down Expand Up @@ -78,36 +75,3 @@ func TestTOKEN_ALL_ACCESS(t *testing.T) {
t.Errorf("TOKEN_ALL_ACCESS = %x, want 0xF01FF", syscall.TOKEN_ALL_ACCESS)
}
}

func TestProcThreadAttributeListPointers(t *testing.T) {
list, err := syscall.NewProcThreadAttributeList(1)
if err != nil {
t.Errorf("unable to create ProcThreadAttributeList: %v", err)
}
done := make(chan struct{})
fds := make([]syscall.Handle, 20)
runtime.SetFinalizer(&fds[0], func(*syscall.Handle) {
close(done)
})
err = syscall.UpdateProcThreadAttribute(list, 0, syscall.PROC_THREAD_ATTRIBUTE_HANDLE_LIST, unsafe.Pointer(&fds[0]), uintptr(len(fds))*unsafe.Sizeof(fds[0]), nil, nil)
if err != nil {
syscall.DeleteProcThreadAttributeList(list)
t.Errorf("unable to update ProcThreadAttributeList: %v", err)
return
}
runtime.GC()
runtime.GC()
select {
case <-done:
t.Error("ProcThreadAttributeList was garbage collected unexpectedly")
default:
}
syscall.DeleteProcThreadAttributeList(list)
runtime.GC()
runtime.GC()
select {
case <-done:
case <-time.After(time.Second):
t.Error("ProcThreadAttributeList was not garbage collected after a second")
}
}
8 changes: 1 addition & 7 deletions src/syscall/types_windows.go
Expand Up @@ -4,8 +4,6 @@

package syscall

import "unsafe"

const (
// Windows errors.
ERROR_FILE_NOT_FOUND Errno = 2
Expand Down Expand Up @@ -493,11 +491,7 @@ type StartupInfo struct {
}

type _PROC_THREAD_ATTRIBUTE_LIST struct {
// This is of type unsafe.Pointer, not of type byte or uintptr, because
// the contents of it is mostly a list of pointers, and in most cases,
// that's a list of pointers to Go-allocated objects. In order to keep
// the GC from collecting these objects, we declare this as unsafe.Pointer.
_ [1]unsafe.Pointer
_ [1]byte
}

const (
Expand Down

0 comments on commit b8e9ec8

Please sign in to comment.