service: account for delete pending windows bug in tunneltracker

Sometimes deleting a service disables it and prepares it for being
deleted, but doesn't actually mark it as pending deletion. Presumably
this is due to a race condition in the service management code.
Workaround this by polling for disabled services, so that we don't wind
up sleeping forever.

Reported-by: Thomas Gschwantner <tharre3@gmail.com>

2 files changed, 35 insertions, 12 deletions

diff --git a/service/tunneltracker.go b/service/tunneltracker.go
index cfc894f4..4403e558 100644
--- a/service/tunneltracker.go
+++ b/service/tunneltracker.go
@@ -11,14 +11,16 @@ import (
 	"golang.org/x/sys/windows/svc"
 	"golang.org/x/sys/windows/svc/mgr"
 	"golang.zx2c4.com/wireguard/windows/conf"
+	"log"
 	"runtime"
 	"sync"
 	"syscall"
+	"time"
 	"unsafe"
 )
 
 //sys notifyServiceStatusChange(service windows.Handle, notifyMask uint32, notifyBuffer uintptr) (status uint32) = advapi32.NotifyServiceStatusChangeW
-//sys sleepEx(milliseconds uint32, alertable bool) (ret uint32, err error) = kernel32.SleepEx
+//sys sleepEx(milliseconds uint32, alertable bool) (ret uint32) = kernel32.SleepEx
 
 const (
 	serviceNotify_CREATED          uint32 = 0x00000080
@@ -35,6 +37,7 @@ const (
 const serviceNotify_STATUS_CHANGE uint32 = 2
 const errorServiceMARKED_FOR_DELETE uint32 = 1072
 const errorServiceNOTIFY_CLIENT_LAGGING uint32 = 1294
+const sleepWAIT_IO_COMPLETION uint32 = 0x000000C0
 
 type serviceStatus struct {
 	serviceType             uint32
@@ -120,7 +123,10 @@ func trackedTunnelsGlobalState() (state TunnelState) {
 }
 
 func trackTunnelService(tunnelName string, service *mgr.Service) {
-	defer service.Close()
+	defer func() {
+		service.Close()
+		log.Printf("[%s] Tunnel service tracker finished", tunnelName)
+	}()
 
 	trackedTunnelsLock.Lock()
 	if _, found := trackedTunnels[tunnelName]; found {
@@ -141,13 +147,37 @@ func trackTunnelService(tunnelName string, service *mgr.Service) {
 		notifyCallback: serviceTrackerCallbackPtr,
 	}
 
+	checkForDisabled := func() (shouldReturn bool) {
+		config, err := service.Config()
+		if err == syscall.Errno(serviceMARKED_FOR_DELETE) || config.StartType == windows.SERVICE_DISABLED {
+			log.Printf("[%s] Found disabled service via timeout, so deleting", tunnelName)
+			service.Delete()
+			trackedTunnelsLock.Lock()
+			trackedTunnels[tunnelName] = TunnelStopped
+			trackedTunnelsLock.Unlock()
+			IPCServerNotifyTunnelChange(tunnelName, TunnelStopped, nil)
+			return true
+		}
+		return false
+	}
+	if checkForDisabled() {
+		return
+	}
+
 	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 	lastState := TunnelUnknown
 	for {
 		ret := notifyServiceStatusChange(service.Handle, serviceNotifications, uintptr(unsafe.Pointer(notifier)))
 		switch ret {
 		case 0:
-			sleepEx(windows.INFINITE, true)
+			for {
+				if sleepEx(uint32(time.Second*3/time.Millisecond), true) == sleepWAIT_IO_COMPLETION {
+					break
+				} else if checkForDisabled() {
+					return
+				}
+			}
 		case errorServiceMARKED_FOR_DELETE:
 			trackedTunnelsLock.Lock()
 			trackedTunnels[tunnelName] = TunnelStopped
diff --git a/service/zsyscall_windows.go b/service/zsyscall_windows.go
index e8e035b7..38983370 100644
--- a/service/zsyscall_windows.go
+++ b/service/zsyscall_windows.go
@@ -270,21 +270,14 @@ func notifyServiceStatusChange(service windows.Handle, notifyMask uint32, notify
 	return
 }
 
-func sleepEx(milliseconds uint32, alertable bool) (ret uint32, err error) {
+func sleepEx(milliseconds uint32, alertable bool) (ret uint32) {
 	var _p0 uint32
 	if alertable {
 		_p0 = 1
 	} else {
 		_p0 = 0
 	}
-	r0, _, e1 := syscall.Syscall(procSleepEx.Addr(), 2, uintptr(milliseconds), uintptr(_p0), 0)
+	r0, _, _ := syscall.Syscall(procSleepEx.Addr(), 2, uintptr(milliseconds), uintptr(_p0), 0)
 	ret = uint32(r0)
-	if ret == 0 {
-		if e1 != 0 {
-			err = errnoErr(e1)
-		} else {
-			err = syscall.EINVAL
-		}
-	}
 	return
 }