windows/svc/mgr: Service.Control: populate Status when returning certain errors

Fixes golang/go#59015

Change-Id: I45f22049f3a05f807f78d20c9ed67c6c79e3d3c1
GitHub-Last-Rev: 929aeb4acb899e813a44dee1e9a441876939493c
GitHub-Pull-Request: golang/sys#156
Reviewed-on: https://go-review.googlesource.com/c/sys/+/484895
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

windows/svc/mgr/mgr_test.go

@@ -17,6 +17,7 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/sys/windows"
 	"golang.org/x/sys/windows/svc"
 	"golang.org/x/sys/windows/svc/mgr"
 )
@@ -109,7 +110,7 @@ func testRecoveryActions(t *testing.T, s *mgr.Service, should []mgr.RecoveryActi
 	if len(should) != len(is) {
 		t.Errorf("recovery action mismatch: contains %v actions, but should have %v", len(is), len(should))
 	}
-	for i, _ := range is {
+	for i := range is {
 		if should[i].Type != is[i].Type {
 			t.Errorf("recovery action mismatch: Type is %v, but should have %v", is[i].Type, should[i].Type)
 		}
@@ -131,19 +132,19 @@ func testResetPeriod(t *testing.T, s *mgr.Service, should uint32) {
 
 func testSetRecoveryActions(t *testing.T, s *mgr.Service) {
 	r := []mgr.RecoveryAction{
-		mgr.RecoveryAction{
+		{
 			Type:  mgr.NoAction,
 			Delay: 60000 * time.Millisecond,
 		},
-		mgr.RecoveryAction{
+		{
 			Type:  mgr.ServiceRestart,
 			Delay: 4 * time.Minute,
 		},
-		mgr.RecoveryAction{
+		{
 			Type:  mgr.ServiceRestart,
 			Delay: time.Minute,
 		},
-		mgr.RecoveryAction{
+		{
 			Type:  mgr.RunCommand,
 			Delay: 4000 * time.Millisecond,
 		},
@@ -208,6 +209,16 @@ func testRecoveryCommand(t *testing.T, s *mgr.Service, should string) {
 	}
 }
 
+func testControl(t *testing.T, s *mgr.Service, c svc.Cmd, expectedErr error, expectedStatus svc.Status) {
+	status, err := s.Control(c)
+	if err != expectedErr {
+		t.Fatalf("Unexpected return from s.Control: %v (expected %v)", err, expectedErr)
+	}
+	if expectedStatus != status {
+		t.Fatalf("Unexpected status from s.Control: %+v (expected %+v)", status, expectedStatus)
+	}
+}
+
 func remove(t *testing.T, s *mgr.Service) {
 	err := s.Delete()
 	if err != nil {
@@ -251,6 +262,7 @@ func TestMyService(t *testing.T) {
 		t.Fatalf("service %s is not installed", name)
 	}
 	defer s.Close()
+	defer s.Delete()
 
 	c.BinaryPathName = exepath
 	c = testConfig(t, s, c)
@@ -293,6 +305,11 @@ func TestMyService(t *testing.T) {
 	testRecoveryCommand(t, s, fmt.Sprintf("sc query %s", name))
 	testRecoveryCommand(t, s, "") // delete recovery command
 
+	expectedStatus := svc.Status{
+		State: svc.Stopped,
+	}
+	testControl(t, s, svc.Stop, windows.ERROR_SERVICE_NOT_ACTIVE, expectedStatus)
+
 	remove(t, s)
 }
 

windows/svc/mgr/service.go

@@ -45,17 +45,25 @@ func (s *Service) Start(args ...string) error {
 	return windows.StartService(s.Handle, uint32(len(args)), p)
 }
 
-// Control sends state change request c to the service s.
+// Control sends state change request c to the service s. It returns the most
+// recent status the service reported to the service control manager, and an
+// error if the state change request was not accepted.
+// Note that the returned service status is only set if the status change
+// request succeeded, or if it failed with error ERROR_INVALID_SERVICE_CONTROL,
+// ERROR_SERVICE_CANNOT_ACCEPT_CTRL, or ERROR_SERVICE_NOT_ACTIVE.
 func (s *Service) Control(c svc.Cmd) (svc.Status, error) {
 	var t windows.SERVICE_STATUS
 	err := windows.ControlService(s.Handle, uint32(c), &t)
-	if err != nil {
+	if err != nil &&
+		err != windows.ERROR_INVALID_SERVICE_CONTROL &&
+		err != windows.ERROR_SERVICE_CANNOT_ACCEPT_CTRL &&
+		err != windows.ERROR_SERVICE_NOT_ACTIVE {
 		return svc.Status{}, err
 	}
 	return svc.Status{
 		State:   svc.State(t.CurrentState),
 		Accepts: svc.Accepted(t.ControlsAccepted),
-	}, nil
+	}, err
 }
 
 // Query returns current status of service s.