[sys] windows/svc: rewrite in Go
146 views
Skip to first unread message
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:04:53 AM6/22/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Jason A. Donenfeld has uploaded this change for review.
windows/svc: rewrite in Go
The old service management code was a speghetti mess of unmaintainable
assembly, communicating over events, with non-obvious control flow.
Rewrite all of this in boring vanilla Go.
==== WIP: see below ====
At the moment, this works on Go 1.17 if you call
`windows.SetServiceStatus` early in the program, before the service main
runs. The reason is that otherwise, it's called from service main, which
results in copystack() being called, because it needs to allocate to
resolve that function. copystack() then calls gentraceback, which
panics. This looks like a 1.17 bug we should fix:
runtime: unexpected return pc for golang.org/x/sys/windows/svc.Run.func1 called from 0x1
stack: frame={sp:0xc000079a10, fp:0xc000079a38} stack=[0xc000076000,0xc00007a000)
0x000000c000079910: 0x0000000000000000 0x0000000000000000
0x000000c000079920: 0x0000000000000000 0x0000000000000001
0x000000c000079930: 0x0000000000000000 0x0000000000000000
0x000000c000079940: 0x0000000000000000 0x0000000000000000
0x000000c000079950: 0x0000000000000000 0x0000000000000000
0x000000c000079960: 0x0000000000000001 0x0000000000000000
0x000000c000079970: 0x0000000000000000 0x0000000000000000
0x000000c000079980: 0x0000000000000000 0x0000000000000000
0x000000c000079990: 0x000000c000018180 0x000000c00008c0c0
0x000000c0000799a0: 0x000000c00008c000 0x000000c00008c060
0x000000c0000799b0: 0x0000000000de8218 0x0000000000000000
0x000000c0000799c0: 0x0000000000000000 0x000000c00008c0c0
0x000000c0000799d0: 0x000000c000079858 0x000000c00008c060
0x000000c0000799e0: 0x000000c0000798bc 0x000000c000018180
0x000000c0000799f0: 0x000000c0000798f8 0x000000c000079870
0x000000c000079a00: 0x000000c000079a28 0x0000000000805465 <golang.org/x/sys/windows/svc.Run.func1+0x0000000000000025>
0x000000c000079a10: <0x000000c0001973e0 0x0000000000000001
0x000000c000079a20: 0x0000019079b46fe8 0x0000000000000001
0x000000c000079a30: !0x0000000000000001 >0x0000000000000000
0x000000c000079a40: 0x0000000000000000 0x000000c000055e28
0x000000c000079a50: 0x00000000007314f4 <runtime.callbackWrap+0x0000000000000134> 0x0000000000000000
0x000000c000079a60: 0x000000c000059ce0 0x000000c000055ac0
0x000000c000079a70: 0x0000000000000000 0x0000000000000010
0x000000c000079a80: 0x000000c000055d18 0x0000000000000001
0x000000c000079a90: 0x0000000000000002 0x0000000000000002
0x000000c000079aa0: 0x0000000000000008 0x0000000000000000
0x000000c000079ab0: 0x0000000000000001 0x0000000000000008
0x000000c000079ac0: 0x0000000000000000 0x0000000000000000
0x000000c000079ad0: 0x0000000000000000 0x0000000000000000
0x000000c000079ae0: 0x0000000000000000 0x0000000000000000
0x000000c000079af0: 0x0000000000000000 0x0000000000000000
0x000000c000079b00: 0x0000000000000000 0x0000000000000000
0x000000c000079b10: 0x0000000000000000 0x0000000000000000
0x000000c000079b20: 0x0000000000000000 0x0000000000000000
0x000000c000079b30: 0x0000000000000000
fatal error: unknown caller pc
runtime stack:
runtime.throw({0xa8d13d, 0xd617a0})
runtime/panic.go:1198 +0x76
runtime.gentraceback(0x40000, 0x0, 0x0, 0x19000000000, 0x0, 0x0, 0x7fffffff, 0xab91e8, 0x0, 0x0)
runtime/traceback.go:274 +0x1956
runtime.copystack(0xc00004c340, 0x4000)
runtime/stack.go:918 +0x293
runtime.newstack()
runtime/stack.go:1095 +0x47d
runtime.morestack()
runtime/asm_amd64.s:461 +0x93
goroutine 17 [copystack, locked to thread]:
runtime.heapBits.forwardOrBoundary({0x1907ef80000, 0x0, 0x2030000, 0x1907ef8ffff}, 0x400)
runtime/mbitmap.go:497 +0x71 fp=0xc000079390 sp=0xc000079388 pc=0x6f4111
runtime.heapBits.initSpan({0x1907ef80000, 0x55418, 0xc0, 0x1907ef8ffff}, 0x400)
runtime/mbitmap.go:763 +0x8e fp=0xc0000793f8 sp=0xc000079390 pc=0x6f4c2e
runtime.(*mcentral).grow(0xc000055488)
runtime/mcentral.go:250 +0x125 fp=0xc000079440 sp=0xc0000793f8 pc=0x6f7905
runtime.(*mcentral).cacheSpan(0xdbaaa0)
runtime/mcentral.go:161 +0x69e fp=0xc0000794b8 sp=0xc000079440 pc=0x6f767e
runtime.(*mcache).refill(0x19079d90a28, 0x7)
runtime/mcache.go:162 +0xaf fp=0xc000079508 sp=0xc0000794b8 pc=0x6f688f
runtime.(*mcache).nextFree(0x19079d90a28, 0x7)
runtime/malloc.go:880 +0x85 fp=0xc000079550 sp=0xc000079508 pc=0x6ec785
runtime.mallocgc(0x11, 0x0, 0x0)
runtime/malloc.go:1071 +0x4e8 fp=0xc0000795d8 sp=0xc000079550 pc=0x6ece08
runtime.makeslicecopy(0xa8c549, 0x10, 0x0, 0xa8c549)
runtime/slice.go:55 +0x89 fp=0xc000079620 sp=0xc0000795d8 pc=0x7297a9
golang.org/x/sys/windows.ByteSliceFromString({0xa8c549, 0x64})
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/syscall.go:43 +0x8f fp=0xc000079660 sp=0xc000079620 pc=0x7e4a4f
golang.org/x/sys/windows.BytePtrFromString(...)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/syscall.go:52
golang.org/x/sys/windows.(*DLL).FindProc(0xc000004618, {0xa8c549, 0xffffffffffffffff})
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/dll_windows.go:80 +0x46 fp=0xc000079720 sp=0xc000079660 pc=0x7e17c6
golang.org/x/sys/windows.(*LazyProc).Find(0xc0001119e0)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/dll_windows.go:309 +0xd1 fp=0xc000079780 sp=0xc000079720 pc=0x7e1d91
golang.org/x/sys/windows.(*LazyProc).mustFind(...)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/dll_windows.go:323
golang.org/x/sys/windows.(*LazyProc).Addr(...)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/dll_windows.go:333
golang.org/x/sys/windows.SetServiceStatus(0x720e00, 0xc0000797e4)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/zsyscall_windows.go:1184 +0x37 fp=0xc0000797d0 sp=0xc000079780 pc=0x7e9d37
golang.org/x/sys/windows/svc.(*service).updateStatus(0xc0000559b8, 0xc000055860, 0x0)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/svc/service.go:193 +0x125 fp=0xc000079810 sp=0xc0000797d0 pc=0x804b65
golang.org/x/sys/windows/svc.serviceMain(0xc0001973e0, 0x1, 0x19079b46fe8)
golang.org/x/s...@v0.0.0-20210616094352-59db8d763f22/windows/svc/service.go:260 +0x4d3 fp=0xc000079a10 sp=0xc000079810 pc=0x8050b3
runtime: unexpected return pc for golang.org/x/sys/windows/svc.Run.func1 called from 0x1
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 50 insertions(+), 424 deletions(-)
diff --git a/windows/service.go b/windows/service.go
index b269850..1a22b3e 100644
--- a/windows/service.go
+++ b/windows/service.go
@@ -235,3 +235,4 @@
//sys NotifyServiceStatusChange(service Handle, notifyMask uint32, notifier *SERVICE_NOTIFY) (ret error) = advapi32.NotifyServiceStatusChangeW
//sys SubscribeServiceChangeNotifications(service Handle, eventType uint32, callback uintptr, callbackCtx uintptr, subscription *uintptr) (ret error) = sechost.SubscribeServiceChangeNotifications?
//sys UnsubscribeServiceChangeNotifications(subscription uintptr) = sechost.UnsubscribeServiceChangeNotifications?
+//sys RegisterServiceCtrlHandlerEx(serviceName *uint16, handlerProc uintptr, context uintptr) (handle Handle, err error) = advapi32.RegisterServiceCtrlHandlerExW
diff --git a/windows/svc/event.go b/windows/svc/event.go
deleted file mode 100644
index 0508e22..0000000
--- a/windows/svc/event.go
+++ /dev/null
@@ -1,48 +0,0 @@
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build windows
-
-package svc
-
-import (
- "errors"
-
- "golang.org/x/sys/windows"
-)
-
-// event represents auto-reset, initially non-signaled Windows event.
-// It is used to communicate between go and asm parts of this package.
-type event struct {
- h windows.Handle
-}
-
-func newEvent() (*event, error) {
- h, err := windows.CreateEvent(nil, 0, 0, nil)
- if err != nil {
- return nil, err
- }
- return &event{h: h}, nil
-}
-
-func (e *event) Close() error {
- return windows.CloseHandle(e.h)
-}
-
-func (e *event) Set() error {
- return windows.SetEvent(e.h)
-}
-
-func (e *event) Wait() error {
- s, err := windows.WaitForSingleObject(e.h, windows.INFINITE)
- switch s {
- case windows.WAIT_OBJECT_0:
- break
- case windows.WAIT_FAILED:
- return err
- default:
- return errors.New("unexpected result from WaitForSingleObject")
- }
- return nil
-}
diff --git a/windows/svc/go12.c b/windows/svc/go12.c
deleted file mode 100644
index 6f1be1f..0000000
--- a/windows/svc/go12.c
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build windows
-// +build !go1.3
-
-// copied from pkg/runtime
-typedef unsigned int uint32;
-typedef unsigned long long int uint64;
-#ifdef _64BIT
-typedef uint64 uintptr;
-#else
-typedef uint32 uintptr;
-#endif
-
-// from sys_386.s or sys_amd64.s
-void ·servicemain(void);
-
-void
-·getServiceMain(uintptr *r)
-{
- *r = (uintptr)·servicemain;
-}
diff --git a/windows/svc/go12.go b/windows/svc/go12.go
deleted file mode 100644
index cd8b913..0000000
--- a/windows/svc/go12.go
+++ /dev/null
@@ -1,11 +0,0 @@
-// Copyright 2014 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build windows
-// +build !go1.3
-
-package svc
-
-// from go12.c
-func getServiceMain(r *uintptr)
diff --git a/windows/svc/go13.go b/windows/svc/go13.go
deleted file mode 100644
index 9d7f3ce..0000000
--- a/windows/svc/go13.go
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2014 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build windows
-// +build go1.3
-
-package svc
-
-import "unsafe"
-
-const ptrSize = 4 << (^uintptr(0) >> 63) // unsafe.Sizeof(uintptr(0)) but an ideal const
-
-// Should be a built-in for unsafe.Pointer?
-func add(p unsafe.Pointer, x uintptr) unsafe.Pointer {
- return unsafe.Pointer(uintptr(p) + x)
-}
-
-// funcPC returns the entry PC of the function f.
-// It assumes that f is a func value. Otherwise the behavior is undefined.
-func funcPC(f interface{}) uintptr {
- return **(**uintptr)(add(unsafe.Pointer(&f), ptrSize))
-}
-
-// from sys_386.s and sys_amd64.s
-func servicectlhandler(ctl uint32) uintptr
-func servicemain(argc uint32, argv **uint16)
-
-func getServiceMain(r *uintptr) {
- *r = funcPC(servicemain)
-}
diff --git a/windows/svc/service.go b/windows/svc/service.go
index 3748528..762b33d 100644
--- a/windows/svc/service.go
+++ b/windows/svc/service.go
@@ -10,8 +10,6 @@
import (
"errors"
- "runtime"
- "syscall"
"unsafe"
"golang.org/x/sys/internal/unsafeheader"
@@ -91,7 +89,6 @@
// Handler is the interface that must be implemented to build Windows service.
type Handler interface {
-
// Execute will be called by the package code at the start of
// the service, and the service will exit once Execute completes.
// Inside Execute you must read service change requests from r and
@@ -106,28 +103,6 @@
Execute(args []string, r <-chan ChangeRequest, s chan<- Status) (svcSpecificEC bool, exitCode uint32)
}
-var (
- // These are used by asm code.
- goWaitsH uintptr
- cWaitsH uintptr
- ssHandle uintptr
- sName *uint16
- sArgc uintptr
- sArgv **uint16
- ctlHandlerExProc uintptr
- cSetEvent uintptr
- cWaitForSingleObject uintptr
- cRegisterServiceCtrlHandlerExW uintptr
-)
-
-func init() {
- k := windows.NewLazySystemDLL("kernel32.dll")
- cSetEvent = k.NewProc("SetEvent").Addr()
- cWaitForSingleObject = k.NewProc("WaitForSingleObject").Addr()
- a := windows.NewLazySystemDLL("advapi32.dll")
- cRegisterServiceCtrlHandlerExW = a.NewProc("RegisterServiceCtrlHandlerExW").Addr()
-}
-
type ctlEvent struct {
cmd Cmd
eventType uint32
@@ -140,34 +115,16 @@
type service struct {
name string
h windows.Handle
- cWaits *event
- goWaits *event
c chan ctlEvent
handler Handler
}
-func newService(name string, handler Handler) (*service, error) {
+func newService(name string, handler Handler) *service {
var s service
- var err error
s.name = name
s.c = make(chan ctlEvent)
s.handler = handler
- s.cWaits, err = newEvent()
- if err != nil {
- return nil, err
- }
- s.goWaits, err = newEvent()
- if err != nil {
- s.cWaits.Close()
- return nil, err
- }
- return &s, nil
-}
-
-func (s *service) close() error {
- s.cWaits.Close()
- s.goWaits.Close()
- return nil
+ return &s
}
type exitCode struct {
@@ -224,23 +181,34 @@
return windows.SetServiceStatus(s.h, &t)
}
-const (
- sysErrSetServiceStatusFailed = uint32(syscall.APPLICATION_ERROR) + iota
- sysErrNewThreadInCallback
-)
+var ctlHandlerCallback = windows.NewCallback(func(ctl, evtype, evdata, context uintptr) uintptr {
+ s := (*service)(unsafe.Pointer(context))
+ e := ctlEvent{cmd: Cmd(ctl), eventType: uint32(evtype), eventData: evdata, context: 123456} // Set context to 123456 to test issue #25660.
+ s.c <- e
+ return 0
+})
-func (s *service) run() {
- s.goWaits.Wait()
- s.h = windows.Handle(ssHandle)
-
- var argv []*uint16
+func serviceMain(s *service, argc uint32, argv **uint16) uintptr {
+ handle, err := windows.RegisterServiceCtrlHandlerEx(windows.StringToUTF16Ptr(s.name), ctlHandlerCallback, uintptr(unsafe.Pointer(s)))
+ if sysErr, ok := err.(windows.Errno); ok {
+ return uintptr(sysErr)
+ } else if err != nil {
+ return uintptr(windows.ERROR_UNKNOWN_EXCEPTION)
+ }
+ s.h = handle
+ ssHandle = handle
+ defer func() {
+ ssHandle = 0
+ windows.CloseHandle(handle)
+ }()
+ var args16 []*uint16
hdr := (*unsafeheader.Slice)(unsafe.Pointer(&argv))
- hdr.Data = unsafe.Pointer(sArgv)
- hdr.Len = int(sArgc)
- hdr.Cap = int(sArgc)
+ hdr.Data = unsafe.Pointer(argv)
+ hdr.Len = int(argc)
+ hdr.Cap = int(argc)
- args := make([]string, len(argv))
- for i, a := range argv {
+ args := make([]string, len(args16))
+ for i, a := range args16 {
args[i] = windows.UTF16PtrToString(a)
}
@@ -279,9 +247,8 @@
case c := <-changesFromHandler:
err := s.updateStatus(&c, &ec)
if err != nil {
- // best suitable error number
- ec.errno = sysErrSetServiceStatusFailed
- if err2, ok := err.(syscall.Errno); ok {
+ ec.errno = uint32(windows.ERROR_EXCEPTION_IN_SERVICE)
+ if err2, ok := err.(windows.Errno); ok {
ec.errno = uint32(err2)
}
break loop
@@ -293,86 +260,28 @@
}
s.updateStatus(&Status{State: Stopped}, &ec)
- s.cWaits.Set()
-}
-func newCallback(fn interface{}) (cb uintptr, err error) {
- defer func() {
- r := recover()
- if r == nil {
- return
- }
- cb = 0
- switch v := r.(type) {
- case string:
- err = errors.New(v)
- case error:
- err = v
- default:
- err = errors.New("unexpected panic in syscall.NewCallback")
- }
- }()
- return syscall.NewCallback(fn), nil
+ return windows.NO_ERROR
}
-// BUG(brainman): There is no mechanism to run multiple services
-// inside one single executable. Perhaps, it can be overcome by
-// using RegisterServiceCtrlHandlerEx Windows api.
-
// Run executes service name by calling appropriate handler function.
func Run(name string, handler Handler) error {
- runtime.LockOSThread()
-
- tid := windows.GetCurrentThreadId()
-
- s, err := newService(name, handler)
- if err != nil {
- return err
- }
-
- ctlHandler := func(ctl, evtype, evdata, context uintptr) uintptr {
- e := ctlEvent{cmd: Cmd(ctl), eventType: uint32(evtype), eventData: evdata, context: context}
- // We assume that this callback function is running on
- // the same thread as Run. Nowhere in MS documentation
- // I could find statement to guarantee that. So putting
- // check here to verify, otherwise things will go bad
- // quickly, if ignored.
- i := windows.GetCurrentThreadId()
- if i != tid {
- e.errno = sysErrNewThreadInCallback
- }
- s.c <- e
- // Always return NO_ERROR (0) for now.
- return windows.NO_ERROR
- }
-
- var svcmain uintptr
- getServiceMain(&svcmain)
+ s := newService(name, handler)
+ serviceMainCallback := windows.NewCallback(func(argc uint32, argv **uint16) uintptr {
+ return serviceMain(s, argc, argv)
+ })
t := []windows.SERVICE_TABLE_ENTRY{
- {ServiceName: syscall.StringToUTF16Ptr(s.name), ServiceProc: svcmain},
+ {ServiceName: windows.StringToUTF16Ptr(s.name), ServiceProc: serviceMainCallback},
{ServiceName: nil, ServiceProc: 0},
}
-
- goWaitsH = uintptr(s.goWaits.h)
- cWaitsH = uintptr(s.cWaits.h)
- sName = t[0].ServiceName
- ctlHandlerExProc, err = newCallback(ctlHandler)
- if err != nil {
- return err
- }
-
- go s.run()
-
- err = windows.StartServiceCtrlDispatcher(&t[0])
- if err != nil {
- return err
- }
- return nil
+ return windows.StartServiceCtrlDispatcher(&t[0])
}
+var ssHandle windows.Handle
+
// StatusHandle returns service status handle. It is safe to call this function
// from inside the Handler.Execute because then it is guaranteed to be set.
// This code will have to change once multiple services are possible per process.
func StatusHandle() windows.Handle {
- return windows.Handle(ssHandle)
+ return ssHandle
}
diff --git a/windows/svc/sys_windows_386.s b/windows/svc/sys_windows_386.s
deleted file mode 100644
index 1ed9141..0000000
--- a/windows/svc/sys_windows_386.s
+++ /dev/null
@@ -1,67 +0,0 @@
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// func servicemain(argc uint32, argv **uint16)
-TEXT ·servicemain(SB),7,$0
- MOVL argc+0(FP), AX
- MOVL AX, ·sArgc(SB)
- MOVL argv+4(FP), AX
- MOVL AX, ·sArgv(SB)
-
- PUSHL BP
- PUSHL BX
- PUSHL SI
- PUSHL DI
-
- SUBL $12, SP
-
- MOVL ·sName(SB), AX
- MOVL AX, (SP)
- MOVL $·servicectlhandler(SB), AX
- MOVL AX, 4(SP)
- // Set context to 123456 to test issue #25660.
- MOVL $123456, 8(SP)
- MOVL ·cRegisterServiceCtrlHandlerExW(SB), AX
- MOVL SP, BP
- CALL AX
- MOVL BP, SP
- CMPL AX, $0
- JE exit
- MOVL AX, ·ssHandle(SB)
-
- MOVL ·goWaitsH(SB), AX
- MOVL AX, (SP)
- MOVL ·cSetEvent(SB), AX
- MOVL SP, BP
- CALL AX
- MOVL BP, SP
-
- MOVL ·cWaitsH(SB), AX
- MOVL AX, (SP)
- MOVL $-1, AX
- MOVL AX, 4(SP)
- MOVL ·cWaitForSingleObject(SB), AX
- MOVL SP, BP
- CALL AX
- MOVL BP, SP
-
-exit:
- ADDL $12, SP
-
- POPL DI
- POPL SI
- POPL BX
- POPL BP
-
- MOVL 0(SP), CX
- ADDL $12, SP
- JMP CX
-
-// I do not know why, but this seems to be the only way to call
-// ctlHandlerProc on Windows 7.
-
-// func servicectlhandler(ctl uint32, evtype uint32, evdata uintptr, context uintptr) uintptr {
-TEXT ·servicectlhandler(SB),7,$0
- MOVL ·ctlHandlerExProc(SB), CX
- JMP CX
diff --git a/windows/svc/sys_windows_amd64.s b/windows/svc/sys_windows_amd64.s
deleted file mode 100644
index 1e5ef92..0000000
--- a/windows/svc/sys_windows_amd64.s
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright 2012 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// func servicemain(argc uint32, argv **uint16)
-TEXT ·servicemain(SB),7,$0
- MOVQ SP, AX
- ANDQ $~15, SP // alignment as per Windows requirement
- SUBQ $48, SP // room for SP and 4 args as per Windows requirement
- // plus one extra word to keep stack 16 bytes aligned
- MOVQ AX, 32(SP)
-
- MOVL CX, ·sArgc(SB)
- MOVQ DX, ·sArgv(SB)
-
- MOVQ ·sName(SB), CX
- MOVQ $·servicectlhandler(SB), DX
- // BUG(pastarmovj): Figure out a way to pass in context in R8.
- // Set context to 123456 to test issue #25660.
- MOVQ $123456, R8
- MOVQ ·cRegisterServiceCtrlHandlerExW(SB), AX
- CALL AX
- CMPQ AX, $0
- JE exit
- MOVQ AX, ·ssHandle(SB)
-
- MOVQ ·goWaitsH(SB), CX
- MOVQ ·cSetEvent(SB), AX
- CALL AX
-
- MOVQ ·cWaitsH(SB), CX
- MOVQ $4294967295, DX
- MOVQ ·cWaitForSingleObject(SB), AX
- CALL AX
-
-exit:
- MOVQ 32(SP), SP
- RET
-
-// I do not know why, but this seems to be the only way to call
-// ctlHandlerProc on Windows 7.
-
-// func ·servicectlhandler(ctl uint32, evtype uint32, evdata uintptr, context uintptr) uintptr {
-TEXT ·servicectlhandler(SB),7,$0
- MOVQ ·ctlHandlerExProc(SB), AX
- JMP AX
diff --git a/windows/svc/sys_windows_arm.s b/windows/svc/sys_windows_arm.s
deleted file mode 100644
index 360b86e..0000000
--- a/windows/svc/sys_windows_arm.s
+++ /dev/null
@@ -1,36 +0,0 @@
-// Copyright 2018 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-#include "textflag.h"
-
-// func servicemain(argc uint32, argv **uint16)
-TEXT ·servicemain(SB),NOSPLIT|NOFRAME,$0
- MOVM.DB.W [R4, R14], (R13) // push {r4, lr}
- MOVW R13, R4
- BIC $0x7, R13 // alignment for ABI
-
- MOVW R0, ·sArgc(SB)
- MOVW R1, ·sArgv(SB)
-
- MOVW ·sName(SB), R0
- MOVW ·ctlHandlerExProc(SB), R1
- MOVW $0, R2
- MOVW ·cRegisterServiceCtrlHandlerExW(SB), R3
- BL (R3)
- CMP $0, R0
- BEQ exit
- MOVW R0, ·ssHandle(SB)
-
- MOVW ·goWaitsH(SB), R0
- MOVW ·cSetEvent(SB), R1
- BL (R1)
-
- MOVW ·cWaitsH(SB), R0
- MOVW $-1, R1
- MOVW ·cWaitForSingleObject(SB), R2
- BL (R2)
-
-exit:
- MOVW R4, R13 // free extra stack space
- MOVM.IA.W (R13), [R4, R15] // pop {r4, pc}
diff --git a/windows/svc/sys_windows_arm64.s b/windows/svc/sys_windows_arm64.s
deleted file mode 100644
index 3ca540e..0000000
--- a/windows/svc/sys_windows_arm64.s
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2018 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-#include "textflag.h"
-
-// func servicemain(argc uint32, argv **uint16)
-TEXT ·servicemain(SB),NOSPLIT|NOFRAME,$0
- MOVD R0, ·sArgc(SB)
- MOVD R1, ·sArgv(SB)
-
- MOVD ·sName(SB), R0
- MOVD ·ctlHandlerExProc(SB), R1
- MOVD $0, R2
- MOVD ·cRegisterServiceCtrlHandlerExW(SB), R3
- BL (R3)
- CMP $0, R0
- BEQ exit
- MOVD R0, ·ssHandle(SB)
-
- MOVD ·goWaitsH(SB), R0
- MOVD ·cSetEvent(SB), R1
- BL (R1)
-
- MOVD ·cWaitsH(SB), R0
- MOVD $-1, R1
- MOVD ·cWaitForSingleObject(SB), R2
- BL (R2)
-
-exit:
- RET
diff --git a/windows/zsyscall_windows.go b/windows/zsyscall_windows.go
index 148de0f..ce31b6a 100644
--- a/windows/zsyscall_windows.go
+++ b/windows/zsyscall_windows.go
@@ -124,6 +124,7 @@
procRegQueryInfoKeyW = modadvapi32.NewProc("RegQueryInfoKeyW")
procRegQueryValueExW = modadvapi32.NewProc("RegQueryValueExW")
procRegisterEventSourceW = modadvapi32.NewProc("RegisterEventSourceW")
+ procRegisterServiceCtrlHandlerExW = modadvapi32.NewProc("RegisterServiceCtrlHandlerExW")
procReportEventW = modadvapi32.NewProc("ReportEventW")
procRevertToSelf = modadvapi32.NewProc("RevertToSelf")
procSetEntriesInAclW = modadvapi32.NewProc("SetEntriesInAclW")
@@ -1044,6 +1045,15 @@
return
}
+func RegisterServiceCtrlHandlerEx(serviceName *uint16, handlerProc uintptr, context uintptr) (handle Handle, err error) {
+ r0, _, e1 := syscall.Syscall(procRegisterServiceCtrlHandlerExW.Addr(), 3, uintptr(unsafe.Pointer(serviceName)), uintptr(handlerProc), uintptr(context))
+ handle = Handle(r0)
+ if handle == 0 {
+ err = errnoErr(e1)
+ }
+ return
+}
+
func ReportEvent(log Handle, etype uint16, category uint16, eventId uint32, usrSId uintptr, numStrings uint16, dataSize uint32, strings **uint16, rawData *byte) (err error) {
r1, _, e1 := syscall.Syscall9(procReportEventW.Addr(), 9, uintptr(log), uintptr(etype), uintptr(category), uintptr(eventId), uintptr(usrSId), uintptr(numStrings), uintptr(dataSize), uintptr(unsafe.Pointer(strings)), uintptr(unsafe.Pointer(rawData)))
if r1 == 0 {
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:05:13 AM6/22/21
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 1:Run-TryBot +1Trust +1
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:15:39 AM6/22/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #2 to this change.
windows/svc: rewrite in Go
The old service management code was a speghetti mess of unmaintainable
assembly, communicating over events, with non-obvious control flow.
Rewrite all of this in boring vanilla Go.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 46 insertions(+), 430 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:15:46 AM6/22/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 2:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:20:38 AM6/22/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #3 to this change.
windows/svc: rewrite in Go
The old service management code was a speghetti mess of unmaintainable
assembly, communicating over events, with non-obvious control flow.
Rewrite all of this in boring vanilla Go.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 55 insertions(+), 428 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 11:20:47 AM6/22/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 3:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 1:42:36 PM6/22/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #4 to this change.
windows/svc: rewrite in Go
The old service management code was a speghetti mess of unmaintainable
assembly, communicating over events, with non-obvious control flow.
Rewrite all of this in boring vanilla Go.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 56 insertions(+), 429 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 22, 2021, 1:42:49 PM6/22/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 4:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:11:16 PM6/25/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #5 to this change.
windows/svc: rewrite in Go
The old service management code was written in assembly and
communicated over Windows events, which resulted in non-obvious
control flow. NewCallback makes it possible to rewrite all of
this in vanilla Go.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 56 insertions(+), 429 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:11:33 PM6/25/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 5:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:13:31 PM6/25/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
Alex - do you remember why you used assembly rather than NewCallback? Hoping I'm not overlooking something here.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Jun 25, 2021, 5:16:03 PM6/25/21
to Jason A. Donenfeld, goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Ian Lance Taylor, Russ Cox.
4 comments:
Patchset:
Alex, is this stuff all in your head enough to review, or do you want me to do an actual review pass?
Some superficial stuff:
File windows/svc/service.go:
Patch Set #4, Line 180:
initCallbacks sync.Oncemove this up two lines, before what it guards
Patch Set #4, Line 190:
var s service // This is, unfortunately, a global, which means only one service per process."s" is quite a short name for a global variable.
variable name length should be a function of its scope size
how about "theService" ?
And this isn't a new restriction, right?
Patch Set #4, Line 192:
func serviceMain(argc uint32, argv **uint16) uintptr {add a comment on this about what this is and who calls it/when?
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:21:01 PM6/25/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Alex Brainman, Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #6 to this change.
windows/svc: rewrite in Go
The old service management code was written in assembly and communicated
over Windows events, which resulted in non-obvious control flow.
NewCallback makes it possible to rewrite all of this in vanilla Go.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
11 files changed, 63 insertions(+), 434 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:21:02 PM6/25/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 5:Run-TryBot +1Trust +1
3 comments:
File windows/svc/service.go:
Patch Set #4, Line 180:
initCallbacks sync.Oncemove this up two lines, before what it guards
Ack
Patch Set #4, Line 190:
var s service // This is, unfortunately, a global, which means only one service per process.
"s" is quite a short name for a global variable. […]
No, not a new restriction. Trying to copy the design of the old one, which comes with a global StatusHandle function too.
Will choose a better name.
Patch Set #4, Line 192:
func serviceMain(argc uint32, argv **uint16) uintptr {add a comment on this about what this is and who calls it/when?
Ack
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 25, 2021, 5:21:13 PM6/25/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
Patch set 6:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Alex Brainman (Gerrit)
Jun 27, 2021, 4:02:02 AM6/27/21
to Jason A. Donenfeld, goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
Alex, is this stuff all in your head enough to review, or do you want me to do an actual review pass […]
Brad,
I will not be reviewing Jason CLs in the future. I decided to spent my spare time somewhere else.
So feel free to take over this review, if you like.
Alex
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 27, 2021, 4:31:52 AM6/27/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
Brad, […]
Harsh. I wonder what motivated that.
Either way, regardless of whether you review this, it would be helpful to know if there was some rationale behind doing this in assembly before, rather than using NewCallback and such. I'm wondering if the service routines present a unique context that isn't properly accounted for with NewCallback, which your assembly was working around somehow. It'd be nice to know that, so that this approach doesn't accidentally introduce a regression, due to lack of knowledge about the original code's motivation.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 27, 2021, 4:36:41 AM6/27/21
to goph...@pubsubhelper.golang.org, Julian Pastarmov, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Alex Brainman, Brad Fitzpatrick, Ian Lance Taylor, Julian Pastarmov, Russ Cox.
1 comment:
Patchset:
+Julian, who may know the original code.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Alex Brainman (Gerrit)
Jun 27, 2021, 5:10:09 AM6/27/21
to Jason A. Donenfeld, goph...@pubsubhelper.golang.org, Julian Pastarmov, Go Bot, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Brad Fitzpatrick, Ian Lance Taylor, Julian Pastarmov, Russ Cox.
1 comment:
Patchset:
Harsh. I wonder what motivated that.
I agree. You don't find my reviews useful. You disagree with pretty much all my comments. So I decided not to waste my time.
Either way, regardless of whether you review this, it would be helpful to know if there was some rationale behind doing this in assembly before, rather than using NewCallback and such. I'm wondering if the service routines present a unique context that isn't properly accounted for with NewCallback, which your assembly was working around somehow. It'd be nice to know that, so that this approach doesn't accidentally introduce a regression, due to lack of knowledge about the original code's motivation.
That code was written long time ago. That is why it is the way it is.
I don't remember the decisions I made over time. Everything important will be in the code itself or in the comments.
Alex
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Julian Pastarmov (Gerrit)
Jun 28, 2021, 12:05:17 PM6/28/21
to Jason A. Donenfeld, goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
I am sory but I won't be of much help here. I added one function to this lib as it was needed to be able to implement the Google Cloud Print project. I mostly followed the surrounding code examples back then so this is all the context I can provide why it looks as it does. I am also not a Go expert to provide any useful opinion on how it should best be done.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 29, 2021, 7:36:41 PM6/29/21
to goph...@pubsubhelper.golang.org, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
I am sory but I won't be of much help here. […]
No problem. I'll try to do a lot of testing on different platforms, I suppose, to make sure this doesn't regress.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Jun 30, 2021, 11:59:17 AM6/30/21
to Jason A. Donenfeld, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Go Bot, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Jason A. Donenfeld, Ian Lance Taylor, Russ Cox.
Patch set 6:Code-Review +2Trust +1
1 comment:
Patchset:
Assuming this still works back to Windows 7, LGTM.
I imagine our builder test coverage here is limited? Maybe that can be fixed in this change? I think our builders run with enough permissions that they could register services.
I notice the test has:
func TestExample(t *testing.T) {
if testing.Short() {
t.Skip("skipping test in short mode - it modifies system services")
}
Perhaps we should force it to be run on the Go builders. (os.Getenv("GO_BUILDER_NAME") != "")Want to try that in this CL for added confidence?
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 30, 2021, 1:06:32 PM6/30/21
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Go Bot, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Ian Lance Taylor, Russ Cox.
1 comment:
Patchset:
Assuming this still works back to Windows 7, LGTM. […]
Good idea. Would automate my own testing.
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 30, 2021, 1:10:02 PM6/30/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Ian Lance Taylor, Russ Cox.
Jason A. Donenfeld uploaded patch set #7 to this change.
windows/svc: rewrite in Go
The old service management code was written in assembly and communicated
over Windows events, which resulted in non-obvious control flow.
NewCallback makes it possible to rewrite all of this in vanilla Go. This
also enables the service test on the Go builders, as modifying system
services shouldn't be an issue there.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
---
M windows/service.go
D windows/svc/event.go
D windows/svc/go12.c
D windows/svc/go12.go
D windows/svc/go13.go
M windows/svc/service.go
M windows/svc/svc_test.go
D windows/svc/sys_windows_386.s
D windows/svc/sys_windows_amd64.s
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_arm64.s
M windows/zsyscall_windows.go
12 files changed, 64 insertions(+), 435 deletions(-)
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Jun 30, 2021, 1:10:17 PM6/30/21
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Go Bot, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Ian Lance Taylor, Russ Cox.
Patch set 7:Run-TryBot +1Trust +1
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Jason A. Donenfeld (Gerrit)
Oct 20, 2021, 2:40:56 AM10/20/21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Brad Fitzpatrick, Alex Brainman, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Jason A. Donenfeld submitted this change.
6 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: windows/svc/svc_test.go
Insertions: 1, Deletions: 1.
@@ -77,7 +77,7 @@
}
func TestExample(t *testing.T) {
- if testing.Short() {
+ if testing.Short() && os.Getenv("GO_BUILDER_NAME") != "" {
t.Skip("skipping test in short mode - it modifies system services")
}
```
Approvals:
Brad Fitzpatrick: Looks good to me, approved; Trusted
Jason A. Donenfeld: Trusted; Run TryBots
Go Bot: TryBots succeeded
windows/svc: rewrite in Go
The old service management code was written in assembly and communicated
over Windows events, which resulted in non-obvious control flow.
NewCallback makes it possible to rewrite all of this in vanilla Go. This
also enables the service test on the Go builders, as modifying system
services shouldn't be an issue there.
Change-Id: I8003b57d11d4469f762058c648a4b7733530eeb8
Reviewed-on: https://go-review.googlesource.com/c/sys/+/330010
Trust: Jason A. Donenfeld <Ja...@zx2c4.com>
Trust: Brad Fitzpatrick <brad...@golang.org>
Run-TryBot: Jason A. Donenfeld <Ja...@zx2c4.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
---
M windows/service.go
D windows/svc/go13.go
D windows/svc/event.go
D windows/svc/go12.go
D windows/svc/sys_windows_arm.s
D windows/svc/sys_windows_amd64.s
M windows/zsyscall_windows.go
D windows/svc/go12.c
M windows/svc/svc_test.go
M windows/svc/service.go
D windows/svc/sys_windows_arm64.s
D windows/svc/sys_windows_386.s
12 files changed, 85 insertions(+), 435 deletions(-)
index 3748528..46c73a5 100644
--- a/windows/svc/service.go
+++ b/windows/svc/service.go
@@ -10,8 +10,7 @@
import (
"errors"
- "runtime"
- "syscall"
+ "sync"
"unsafe"
"golang.org/x/sys/internal/unsafeheader"
@@ -91,7 +90,6 @@
// Handler is the interface that must be implemented to build Windows service.
type Handler interface {
-
// Execute will be called by the package code at the start of
// the service, and the service will exit once Execute completes.
// Inside Execute you must read service change requests from r and
@@ -106,28 +104,6 @@@@ -140,36 +116,10 @@
type service struct {
name string
h windows.Handle
- cWaits *event
- goWaits *event
c chan ctlEvent
handler Handler
}
-func newService(name string, handler Handler) (*service, error) {
- var s service
- var err error
- s.name = name
- s.c = make(chan ctlEvent)
- s.handler = handler
- s.cWaits, err = newEvent()
- if err != nil {
- return nil, err
- }
- s.goWaits, err = newEvent()
- if err != nil {
- s.cWaits.Close()
- return nil, err
- }
- return &s, nil
-}
-
-func (s *service) close() error {
- s.cWaits.Close()
- s.goWaits.Close()
- return nil
-}
-
type exitCode struct {
isSvcSpecific bool
errno uint32
@@ -224,23 +174,43 @@
return windows.SetServiceStatus(s.h, &t)
}
-const (
- sysErrSetServiceStatusFailed = uint32(syscall.APPLICATION_ERROR) + iota
- sysErrNewThreadInCallback
+var (
+ initCallbacks sync.Once
+ ctlHandlerCallback uintptr
+ serviceMainCallback uintptr
)
-func (s *service) run() {
- s.goWaits.Wait()
- s.h = windows.Handle(ssHandle)
+func ctlHandler(ctl, evtype, evdata, context uintptr) uintptr {
+ s := (*service)(unsafe.Pointer(context))
+ e := ctlEvent{cmd: Cmd(ctl), eventType: uint32(evtype), eventData: evdata, context: 123456} // Set context to 123456 to test issue #25660.
+ s.c <- e
+ return 0
+}
- var argv []*uint16
- hdr := (*unsafeheader.Slice)(unsafe.Pointer(&argv))
- hdr.Data = unsafe.Pointer(sArgv)
- hdr.Len = int(sArgc)
- hdr.Cap = int(sArgc)
+var theService service // This is, unfortunately, a global, which means only one service per process.
- args := make([]string, len(argv))
- for i, a := range argv {
+// serviceMain is the entry point called by the service manager, registered earlier by
+// the call to StartServiceCtrlDispatcher.
+func serviceMain(argc uint32, argv **uint16) uintptr {
+ handle, err := windows.RegisterServiceCtrlHandlerEx(windows.StringToUTF16Ptr(theService.name), ctlHandlerCallback, uintptr(unsafe.Pointer(&theService)))
+ if sysErr, ok := err.(windows.Errno); ok {
+ return uintptr(sysErr)
+ } else if err != nil {
+ return uintptr(windows.ERROR_UNKNOWN_EXCEPTION)
+ }
+ theService.h = handle
+ defer func() {
+ theService.h = 0
+ windows.CloseHandle(handle)
+ }()
+ var args16 []*uint16
+ hdr := (*unsafeheader.Slice)(unsafe.Pointer(&args16))
+ hdr.Data = unsafe.Pointer(argv)
+ hdr.Len = int(argc)
+ hdr.Cap = int(argc)
+
+ args := make([]string, len(args16))
+ for i, a := range args16 {
args[i] = windows.UTF16PtrToString(a)
}
@@ -249,7 +219,7 @@
exitFromHandler := make(chan exitCode)
go func() {
- ss, errno := s.handler.Execute(args, cmdsToHandler, changesFromHandler)
+ ss, errno := theService.handler.Execute(args, cmdsToHandler, changesFromHandler)
exitFromHandler <- exitCode{ss, errno}
}()
@@ -258,7 +228,7 @@
CurrentStatus: Status{State: Stopped},
}
var outch chan ChangeRequest
- inch := s.c
+ inch := theService.c
loop:
for {
select {
@@ -274,14 +244,13 @@
outcr.EventData = r.eventData
outcr.Context = r.context
case outch <- outcr:
- inch = s.c
+ inch = theService.c
outch = nil
case c := <-changesFromHandler:
- err := s.updateStatus(&c, &ec)
+ err := theService.updateStatus(&c, &ec)
if err != nil {
- // best suitable error number
- ec.errno = sysErrSetServiceStatusFailed
- if err2, ok := err.(syscall.Errno); ok {
+ ec.errno = uint32(windows.ERROR_EXCEPTION_IN_SERVICE)
+ if err2, ok := err.(windows.Errno); ok {
ec.errno = uint32(err2)
}
break loop
@@ -292,87 +261,30 @@
}
}
- s.updateStatus(&Status{State: Stopped}, &ec)
- s.cWaits.Set()
-}
+ theService.updateStatus(&Status{State: Stopped}, &ec)
+ initCallbacks.Do(func() {
+ ctlHandlerCallback = windows.NewCallback(ctlHandler)
+ serviceMainCallback = windows.NewCallback(serviceMain)
+ })
+ theService.name = name
+ theService.handler = handler
+ theService.c = make(chan ctlEvent)
t := []windows.SERVICE_TABLE_ENTRY{
- {ServiceName: syscall.StringToUTF16Ptr(s.name), ServiceProc: svcmain},
+ {ServiceName: windows.StringToUTF16Ptr(theService.name), ServiceProc: serviceMainCallback},
{ServiceName: nil, ServiceProc: 0},
}
-
- goWaitsH = uintptr(s.goWaits.h)
- cWaitsH = uintptr(s.cWaits.h)
- sName = t[0].ServiceName
- ctlHandlerExProc, err = newCallback(ctlHandler)
- if err != nil {
- return err
- }
-
- go s.run()
-
- err = windows.StartServiceCtrlDispatcher(&t[0])
- if err != nil {
- return err
- }
- return nil
+ return windows.StartServiceCtrlDispatcher(&t[0])
}
// StatusHandle returns service status handle. It is safe to call this function
// from inside the Handler.Execute because then it is guaranteed to be set.
// This code will have to change once multiple services are possible per process.
func StatusHandle() windows.Handle {
- return windows.Handle(ssHandle)
+ return theService.h
}
diff --git a/windows/svc/svc_test.go b/windows/svc/svc_test.go
index e8684dd..f7833ad 100644
--- a/windows/svc/svc_test.go
+++ b/windows/svc/svc_test.go
@@ -77,7 +77,7 @@
}
func TestExample(t *testing.T) {
- if testing.Short() {
+ if testing.Short() && os.Getenv("GO_BUILDER_NAME") != "" {
t.Skip("skipping test in short mode - it modifies system services")
}
index 4ea788e..1636013 100644
--- a/windows/zsyscall_windows.go
+++ b/windows/zsyscall_windows.go
@@ -124,6 +124,7 @@
procRegQueryInfoKeyW = modadvapi32.NewProc("RegQueryInfoKeyW")
procRegQueryValueExW = modadvapi32.NewProc("RegQueryValueExW")
procRegisterEventSourceW = modadvapi32.NewProc("RegisterEventSourceW")
+ procRegisterServiceCtrlHandlerExW = modadvapi32.NewProc("RegisterServiceCtrlHandlerExW")
procReportEventW = modadvapi32.NewProc("ReportEventW")
procRevertToSelf = modadvapi32.NewProc("RevertToSelf")
procSetEntriesInAclW = modadvapi32.NewProc("SetEntriesInAclW")
@@ -1055,6 +1056,15 @@
return
}
+func RegisterServiceCtrlHandlerEx(serviceName *uint16, handlerProc uintptr, context uintptr) (handle Handle, err error) {
+ r0, _, e1 := syscall.Syscall(procRegisterServiceCtrlHandlerExW.Addr(), 3, uintptr(unsafe.Pointer(serviceName)), uintptr(handlerProc), uintptr(context))
+ handle = Handle(r0)
+ if handle == 0 {
+ err = errnoErr(e1)
+ }
+ return
+}
+
func ReportEvent(log Handle, etype uint16, category uint16, eventId uint32, usrSId uintptr, numStrings uint16, dataSize uint32, strings **uint16, rawData *byte) (err error) {
r1, _, e1 := syscall.Syscall9(procReportEventW.Addr(), 9, uintptr(log), uintptr(etype), uintptr(category), uintptr(eventId), uintptr(usrSId), uintptr(numStrings), uintptr(dataSize), uintptr(unsafe.Pointer(strings)), uintptr(unsafe.Pointer(rawData)))
if r1 == 0 {
To view, visit change 330010. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages