From 383cbadff5d9550338822ce465c20c1775a7ee21 Mon Sep 17 00:00:00 2001
From: Torben Schmitz <torsm@google.com>
Date: Mon, 26 Aug 2024 06:49:21 -0700
Subject: [PATCH] Remove the stderrthreshold argument from the windows service
 config

Stderr gets disabled when running as a Windows service now, there is no need for this workaround anymore.

PiperOrigin-RevId: 667568549
---
 fleetspeak/client-win/fleetspeak_lib.wxs     |  4 +--
 fleetspeak/src/client/entry/entry_windows.go | 35 +++++++++-----------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/fleetspeak/client-win/fleetspeak_lib.wxs b/fleetspeak/client-win/fleetspeak_lib.wxs
index ec084c46..d72db481 100644
--- a/fleetspeak/client-win/fleetspeak_lib.wxs
+++ b/fleetspeak/client-win/fleetspeak_lib.wxs
@@ -22,10 +22,8 @@
             Source="$(var.FLEETSPEAK_EXECUTABLE)"
             Checksum="yes"
             KeyPath="yes" />
-        <!-- -stderrthreshold=3 turns off logging to stderr, which would
-             kill the Windows service. -->
         <ServiceInstall
-            Arguments="-config=[FleetspeakInstallDir]fleetspeak-client.config -stderrthreshold=3 -log_dir=[%SYSTEMROOT]\Temp\Fleetspeak"
+            Arguments="-config=[FleetspeakInstallDir]fleetspeak-client.config -log_dir=[%SYSTEMROOT]\Temp\Fleetspeak"
             Id="ServiceInstaller"
             Type="ownProcess"
             Name="FleetspeakClientService"
diff --git a/fleetspeak/src/client/entry/entry_windows.go b/fleetspeak/src/client/entry/entry_windows.go
index b78a5a4c..3dfcebfb 100644
--- a/fleetspeak/src/client/entry/entry_windows.go
+++ b/fleetspeak/src/client/entry/entry_windows.go
@@ -5,7 +5,6 @@ package entry
 import (
 	"context"
 	"errors"
-	"flag"
 	"os"
 	"os/signal"
 	"sync"
@@ -25,14 +24,7 @@ func (m *fleetspeakService) Execute(args []string, r <-chan svc.ChangeRequest, c
 	const cmdsAccepted = svc.AcceptStop | svc.AcceptShutdown
 	changes <- svc.Status{State: svc.StartPending}
 	changes <- svc.Status{State: svc.Running, Accepts: cmdsAccepted}
-
-	// stderr is not available when running as a Windows service, and writing to
-	// it causes the log library to exit.
-	//
-	// We work around this by setting the "stderrthreshold" flag to "FATAL",
-	// which ensures that ERROR logs don't get written to stderr. Only FATAL logs
-	// write to stderr, which is fine because on fatal errors we exit anyway.
-	flag.Set("stderrthreshold", "FATAL")
+	tryDisableStderr()
 	log.Info("Service started.")
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -73,7 +65,7 @@ func (m *fleetspeakService) Execute(args []string, r <-chan svc.ChangeRequest, c
 	// return an error, Windows doesn't seem to consider this orderly-shutdown an
 	// error, so it doesn't restart us. Hence if there's an error we exit.
 	if err != nil {
-		exitf("Stopped due to unrecoverable error: %v", err)
+		log.Exitf("Stopped due to unrecoverable error: %v", err)
 	}
 
 	log.Info("Successfully stopped service.")
@@ -88,7 +80,7 @@ func (m *fleetspeakService) ExecuteAsRegularProcess() {
 
 	err := m.innerMain(ctx)
 	if err != nil {
-		exitf("Stopped due to unrecoverable error: %v", err)
+		log.Exitf("Stopped due to unrecoverable error: %v", err)
 	}
 }
 
@@ -104,7 +96,7 @@ func RunMain(innerMain InnerMain, windowsServiceName string) {
 		log.Info("Not running as a service, executing as a regular process.")
 		fs.ExecuteAsRegularProcess()
 	} else if err != nil {
-		exitf("Failed to run service: %v", err)
+		log.Exitf("Failed to run service: %v", err)
 	}
 }
 
@@ -112,16 +104,19 @@ func enforceShutdownTimeout(ctx context.Context) {
 	context.AfterFunc(ctx, func() {
 		log.Info("Main context stopped, shutting down...")
 		time.AfterFunc(shutdownTimeout, func() {
-			exitf("Fleetspeak failed to shut down in %v. Exiting ungracefully.", shutdownTimeout)
+			log.Exitf("Fleetspeak failed to shut down in %v. Exiting ungracefully.", shutdownTimeout)
 		})
 	})
 }
 
-// exitf logs an error and exits with a non-zero code.
-// This is a replacement for log.Exitf which logs at FATAL level, and therefore
-// attempts to write to stderr, which is not available when running as a Windows
-// service.
-func exitf(format string, args ...any) {
-	log.Errorf(format, args...)
-	os.Exit(1)
+// tryDisableStderr redirects [os.Stderr] to [os.DevNull]. When running as a
+// Windows service, stderr is not available, causing the logging library to
+// crash when attempting to write an error log.
+func tryDisableStderr() {
+	devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0)
+	if err != nil {
+		log.Warningf("Failed to disable stderr, the application might shut down ungracefully: %v", os.DevNull, err)
+		return
+	}
+	os.Stderr = devNull
 }