From 12036c158aa716b5583dcf66cd179e6acb32014e Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sun, 20 Jan 2019 15:45:04 +1100 Subject: [PATCH] windows/svc: add Context to ChangeRequest New Context field will be used in the following CL to test ctlHandler parameter alignments. Also adjust TestExample to pass hard coded Context value of 123456 to test service, and verify that correct value is logged. Final part of the test is commented out, and will be adjusted in the next CL. Updates golang/go#25660 Change-Id: Iad2896ae497ee1edc0d62655eaf08671ec2651c5 Reviewed-on: https://go-review.googlesource.com/c/158697 Run-TryBot: Alex Brainman TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- windows/svc/example/service.go | 5 ++++- windows/svc/service.go | 24 ++++++++++++------------ windows/svc/svc_test.go | 6 +++++- windows/svc/sys_386.s | 3 ++- windows/svc/sys_amd64.s | 2 ++ 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/windows/svc/example/service.go b/windows/svc/example/service.go index 74c93930..45e4c90f 100644 --- a/windows/svc/example/service.go +++ b/windows/svc/example/service.go @@ -27,7 +27,6 @@ func (m *myservice) Execute(args []string, r <-chan svc.ChangeRequest, changes c slowtick := time.Tick(2 * time.Second) tick := fasttick changes <- svc.Status{State: svc.Running, Accepts: cmdsAccepted} - elog.Info(1, strings.Join(args, "-")) loop: for { select { @@ -42,6 +41,10 @@ loop: time.Sleep(100 * time.Millisecond) changes <- c.CurrentStatus case svc.Stop, svc.Shutdown: + // golang.org/x/sys/windows/svc.TestExample is verifying this output. + testOutput := strings.Join(args, "-") + testOutput += fmt.Sprintf("-%d", c.Context) + elog.Info(1, testOutput) break loop case svc.Pause: changes <- svc.Status{State: svc.Paused, Accepts: cmdsAccepted} diff --git a/windows/svc/service.go b/windows/svc/service.go index cda26b54..d6ec7e78 100644 --- a/windows/svc/service.go +++ b/windows/svc/service.go @@ -80,6 +80,7 @@ type ChangeRequest struct { EventType uint32 EventData uintptr CurrentStatus Status + Context uintptr } // Handler is the interface that must be implemented to build Windows service. @@ -121,12 +122,11 @@ func init() { cRegisterServiceCtrlHandlerExW = a.MustFindProc("RegisterServiceCtrlHandlerExW").Addr() } -// The HandlerEx prototype also has a context pointer but since we don't use -// it at start-up time we don't have to pass it over either. type ctlEvent struct { cmd Cmd eventType uint32 eventData uintptr + context uintptr errno uint32 } @@ -238,13 +238,12 @@ func (s *service) run() { exitFromHandler <- exitCode{ss, errno} }() - status := Status{State: Stopped} ec := exitCode{isSvcSpecific: true, errno: 0} + outcr := ChangeRequest{ + CurrentStatus: Status{State: Stopped}, + } var outch chan ChangeRequest inch := s.c - var cmd Cmd - var evtype uint32 - var evdata uintptr loop: for { select { @@ -255,10 +254,11 @@ loop: } inch = nil outch = cmdsToHandler - cmd = r.cmd - evtype = r.eventType - evdata = r.eventData - case outch <- ChangeRequest{cmd, evtype, evdata, status}: + outcr.Cmd = r.cmd + outcr.EventType = r.eventType + outcr.EventData = r.eventData + outcr.Context = r.context + case outch <- outcr: inch = s.c outch = nil case c := <-changesFromHandler: @@ -271,7 +271,7 @@ loop: } break loop } - status = c + outcr.CurrentStatus = c case ec = <-exitFromHandler: break loop } @@ -316,7 +316,7 @@ func Run(name string, handler Handler) error { } ctlHandler := func(ctl uint32, evtype uint32, evdata uintptr, context uintptr) uintptr { - e := ctlEvent{cmd: Cmd(ctl), eventType: evtype, eventData: evdata} + e := ctlEvent{cmd: Cmd(ctl), eventType: 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 diff --git a/windows/svc/svc_test.go b/windows/svc/svc_test.go index feed8fab..8e59c0de 100644 --- a/windows/svc/svc_test.go +++ b/windows/svc/svc_test.go @@ -125,7 +125,11 @@ func TestExample(t *testing.T) { if err != nil { t.Fatalf("wevtutil failed: %v\n%v", err, string(out)) } - if want := strings.Join(append([]string{name}, args...), "-"); !strings.Contains(string(out), want) { + want := strings.Join(append([]string{name}, args...), "-") + // Test context passing (see servicemain in sys_386.s and sys_amd64.s). + // TODO(brainman): Uncomment next line once issue #25660 is fixed. + //want += "-123456" + if !strings.Contains(string(out), want) { t.Errorf("%q string does not contain %q", string(out), want) } } diff --git a/windows/svc/sys_386.s b/windows/svc/sys_386.s index 2c82a9d9..c8a583d7 100644 --- a/windows/svc/sys_386.s +++ b/windows/svc/sys_386.s @@ -22,7 +22,8 @@ TEXT ·servicemain(SB),7,$0 MOVL AX, (SP) MOVL $·servicectlhandler(SB), AX MOVL AX, 4(SP) - MOVL $0, 8(SP) + // Set context to 123456 to test issue #25660. + MOVL $123456, 8(SP) MOVL ·cRegisterServiceCtrlHandlerExW(SB), AX MOVL SP, BP CALL AX diff --git a/windows/svc/sys_amd64.s b/windows/svc/sys_amd64.s index bde25e9c..2f7609c5 100644 --- a/windows/svc/sys_amd64.s +++ b/windows/svc/sys_amd64.s @@ -14,6 +14,8 @@ TEXT ·servicemain(SB),7,$0 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