]> Cypherpunks repositories - gostls13.git/commitdiff
syscall: use runtime.KeepAlive for ProcThreadAttributeList arguments
authorJason A. Donenfeld <Jason@zx2c4.com>
Wed, 10 Mar 2021 14:21:56 +0000 (07:21 -0700)
committerJason A. Donenfeld <Jason@zx2c4.com>
Thu, 11 Mar 2021 13:49:01 +0000 (13:49 +0000)
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>
src/syscall/exec_windows.go
src/syscall/syscall_windows.go
src/syscall/syscall_windows_test.go
src/syscall/types_windows.go

index b20a27d28b8fa590fe29f6ca658672da98e0f779..253e9e8c1f9604997f8de7c24d6adcd57fee0907 100644 (file)
@@ -7,6 +7,7 @@
 package syscall
 
 import (
+       "runtime"
        "sync"
        "unicode/utf16"
        "unsafe"
@@ -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
 }
index 05a7d3027d3904ab3ad6daa53324c0b284f74cd9..65af6637aefaaff37ff64bae890e5e22a104ba39 100644 (file)
@@ -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
index d5e8d58b5a2893785d9eccf48bf4b02368bdb164..a9ae54752b30a5a10516c0dd8b5c991f3b61004a 100644 (file)
@@ -7,11 +7,8 @@ package syscall_test
 import (
        "os"
        "path/filepath"
-       "runtime"
        "syscall"
        "testing"
-       "time"
-       "unsafe"
 )
 
 func TestWin32finddata(t *testing.T) {
@@ -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")
-       }
-}
index 31fe7664c93bb19964cf1234a74798d6dbd7a393..384b5b4f2c1f6ce12361d0c9aa5fb7bf685222cb 100644 (file)
@@ -4,8 +4,6 @@
 
 package syscall
 
-import "unsafe"
-
 const (
        // Windows errors.
        ERROR_FILE_NOT_FOUND      Errno = 2
@@ -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 (