Merge remote-tracking branch 'origin/de/zen-service-command' into de/zen-service-command

20 files changed, 172 insertions, 29 deletions

diff --git a/src/zen/cmds/service_cmd.cpp b/src/zen/cmds/service_cmd.cpp
index 372fce5cb..b3872dae7 100644
--- a/src/zen/cmds/service_cmd.cpp
+++ b/src/zen/cmds/service_cmd.cpp
@@ -160,6 +160,13 @@ ServiceCommand::ServiceCommand()
 								fmt::format("Service name, defaults to \"{}\"", m_ServiceName),
 								cxxopts::value(m_ServiceName),
 								"<name>");
+
+	m_InstallOptions.add_option("",
+								"u",
+								"user",
+								"User to run service as, defaults to current user",
+								cxxopts::value(m_UserName),
+								"<user>");
 #if ZEN_PLATFORM_WINDOWS
 	m_InstallOptions.add_option("",
 								"d",
@@ -340,7 +347,7 @@ ServiceCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		Ec				   = InstallService(
 			m_ServiceName,
 			ServiceSpec {
-				.ExecutablePath = m_ServerExecutable, .CommandLineOptions = GlobalOptions.PassthroughCommandLine
+				.ExecutablePath = m_ServerExecutable, .CommandLineOptions = GlobalOptions.PassthroughCommandLine, .UserName = m_UserName
 #if ZEN_PLATFORM_WINDOWS
 					,
 				.DisplayName = m_ServiceDisplayName, .Description = m_ServiceDescription
diff --git a/src/zen/cmds/service_cmd.h b/src/zen/cmds/service_cmd.h
index 4d370b29c..f88e8c25b 100644
--- a/src/zen/cmds/service_cmd.h
+++ b/src/zen/cmds/service_cmd.h
@@ -26,6 +26,7 @@ private:
 	std::string m_Verb;	 // create, info, remove
 
 	std::string m_ServiceName = "ZenServer";
+	std::string m_UserName;
 
 	bool m_AllowElevation = false;
 
diff --git a/src/zencore-test/zencore-test.cpp b/src/zencore-test/zencore-test.cpp
index 40cb51156..37ae7f587 100644
--- a/src/zencore-test/zencore-test.cpp
+++ b/src/zencore-test/zencore-test.cpp
@@ -12,6 +12,7 @@
 #if ZEN_WITH_TESTS
 #	define ZEN_TEST_WITH_RUNNER 1
 #	include <zencore/testing.h>
+#	include <zencore/process.h>
 #endif
 
 int
@@ -20,6 +21,10 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 #if ZEN_WITH_TESTS
 	zen::zencore_forcelinktests();
 
+#if ZEN_PLATFORM_LINUX
+	zen::IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 	zen::MaximizeOpenFileCount();
 
diff --git a/src/zencore/include/zencore/memory/fmalloc.h b/src/zencore/include/zencore/memory/fmalloc.h
index aeb05b651..5b476429e 100644
--- a/src/zencore/include/zencore/memory/fmalloc.h
+++ b/src/zencore/include/zencore/memory/fmalloc.h
@@ -2,6 +2,8 @@
 
 #pragma once
 
+#include <cstddef>
+
 #include <zenbase/zenbase.h>
 
 namespace zen {
diff --git a/src/zencore/include/zencore/process.h b/src/zencore/include/zencore/process.h
index 42b997c39..36c2a2481 100644
--- a/src/zencore/include/zencore/process.h
+++ b/src/zencore/include/zencore/process.h
@@ -101,6 +101,10 @@ int				 GetProcessId(CreateProcResult ProcId);
 std::filesystem::path GetProcessExecutablePath(int Pid, std::error_code& OutEc);
 std::error_code		  FindProcess(const std::filesystem::path& ExecutableImage, ProcessHandle& OutHandle);
 
+#if ZEN_PLATFORM_LINUX
+void IgnoreChildSignals();
+#endif
+
 void process_forcelink();  // internal
 
 }  // namespace zen
diff --git a/src/zencore/memory/mallocmimalloc.cpp b/src/zencore/memory/mallocmimalloc.cpp
index 1919af3bf..1f9aff404 100644
--- a/src/zencore/memory/mallocmimalloc.cpp
+++ b/src/zencore/memory/mallocmimalloc.cpp
@@ -1,5 +1,7 @@
 // Copyright Epic Games, Inc. All Rights Reserved.
 
+#include <cstring>
+
 #include <zencore/intmath.h>
 #include <zencore/memory/align.h>
 #include <zencore/memory/mallocmimalloc.h>
diff --git a/src/zencore/process.cpp b/src/zencore/process.cpp
index 004b36dca..079e2db3f 100644
--- a/src/zencore/process.cpp
+++ b/src/zencore/process.cpp
@@ -24,7 +24,6 @@
 #	include <sys/sem.h>
 #	include <sys/stat.h>
 #	include <sys/syscall.h>
-#	include <sys/sysctl.h>
 #	include <sys/wait.h>
 #	include <time.h>
 #	include <unistd.h>
@@ -41,7 +40,9 @@ ZEN_THIRD_PARTY_INCLUDES_END
 namespace zen {
 
 #if ZEN_PLATFORM_LINUX
-const bool bNoZombieChildren = []() {
+void
+IgnoreChildSignals()
+{
 	// When a child process exits it is put into a zombie state until the parent
 	// collects its result. This doesn't fit the Windows-like model that Zen uses
 	// where there is a less strict familial model and no zombification. Ignoring
@@ -52,8 +53,7 @@ const bool bNoZombieChildren = []() {
 	sigemptyset(&Action.sa_mask);
 	Action.sa_handler = SIG_IGN;
 	sigaction(SIGCHLD, &Action, nullptr);
-	return true;
-}();
+}
 
 static char
 GetPidStatus(int Pid, std::error_code& OutEc)
diff --git a/src/zenhttp-test/zenhttp-test.cpp b/src/zenhttp-test/zenhttp-test.cpp
index 49db1ba54..df395939b 100644
--- a/src/zenhttp-test/zenhttp-test.cpp
+++ b/src/zenhttp-test/zenhttp-test.cpp
@@ -8,6 +8,7 @@
 #if ZEN_WITH_TESTS
 #	define ZEN_TEST_WITH_RUNNER 1
 #	include <zencore/testing.h>
+#	include <zencore/process.h>
 #endif
 
 int
@@ -16,6 +17,10 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 #if ZEN_WITH_TESTS
 	zen::zenhttp_forcelinktests();
 
+#if ZEN_PLATFORM_LINUX
+	zen::IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 	zen::MaximizeOpenFileCount();
 
diff --git a/src/zennet-test/zennet-test.cpp b/src/zennet-test/zennet-test.cpp
index 482d3c617..b45a5f807 100644
--- a/src/zennet-test/zennet-test.cpp
+++ b/src/zennet-test/zennet-test.cpp
@@ -9,6 +9,7 @@
 #if ZEN_WITH_TESTS
 #	define ZEN_TEST_WITH_RUNNER 1
 #	include <zencore/testing.h>
+#	include <zencore/process.h>
 #endif
 
 int
@@ -17,6 +18,10 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char** argv)
 #if ZEN_WITH_TESTS
 	zen::zennet_forcelinktests();
 
+#if ZEN_PLATFORM_LINUX
+	zen::IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 	zen::MaximizeOpenFileCount();
 
diff --git a/src/zenserver-test/zenserver-test.cpp b/src/zenserver-test/zenserver-test.cpp
index 6259c0f37..5c245d8bb 100644
--- a/src/zenserver-test/zenserver-test.cpp
+++ b/src/zenserver-test/zenserver-test.cpp
@@ -100,6 +100,10 @@ main(int argc, char** argv)
 	using namespace std::literals;
 	using namespace zen;
 
+#if ZEN_PLATFORM_LINUX
+	IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 
 	zen::logging::SetLogLevel(zen::logging::level::Debug);
diff --git a/src/zenserver/main.cpp b/src/zenserver/main.cpp
index f35010866..9ae54bdf1 100644
--- a/src/zenserver/main.cpp
+++ b/src/zenserver/main.cpp
@@ -23,6 +23,8 @@
 #include <zencore/memory/memorytrace.h>
 #include <zencore/memory/newdelete.h>
 
+#include <zenutil/service.h>
+
 #include "config.h"
 #include "diag/logging.h"
 #include "sentryintegration.h"
@@ -32,6 +34,10 @@
 #	include "windows/service.h"
 #endif
 
+#if ZEN_PLATFORM_LINUX
+#	include <systemd/sd-daemon.h>
+#endif
+
 //////////////////////////////////////////////////////////////////////////
 // We don't have any doctest code in this file but this is needed to bring
 // in some shared code into the executable
@@ -88,6 +94,43 @@ ZenEntryPoint::ZenEntryPoint(ZenServerOptions& ServerOptions) : m_ServerOptions(
 {
 }
 
+void
+ReportServiceStatus(ServiceStatus Status)
+{
+#if ZEN_PLATFORM_WINDOWS
+	switch (Status)
+	{
+		case ServiceStatus::Starting:
+			ReportSvcStatus(SERVICE_START_PENDING, NO_ERROR, 3000);
+			break;
+		case ServiceStatus::Running:
+			ReportSvcStatus(SERVICE_RUNNING, NO_ERROR, 0);
+			break;
+		case ServiceStatus::Stopping:
+			ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0);
+			break;
+		case ServiceStatus::Stopped:
+			ReportSvcStatus(SERVICE_STOPPED, (DWORD)ApplicationExitCode(), 0);
+			break;
+		default:
+			break;
+	}
+#elif ZEN_PLATFORM_LINUX
+	switch (Status)
+	{
+		case ServiceStatus::Running:
+			sd_notify(0, "READY=1");
+			break;
+		case ServiceStatus::Stopping:
+			sd_notify(0, "STOPPING=1");
+			break;
+		case ServiceStatus::Stopped:
+			sd_notifyf(0, "EXIT_STATUS=%d", ApplicationExitCode());
+			break;
+	}
+#endif
+}
+
 int
 ZenEntryPoint::Run()
 {
@@ -281,6 +324,8 @@ ZenEntryPoint::Run()
 		}});
 
 		auto CleanupShutdown = MakeGuard([&ShutdownEvent, &ShutdownThread] {
+			ReportServiceStatus(ServiceStatus::Stopping);
+
 			if (ShutdownEvent)
 			{
 				ShutdownEvent->Set();
@@ -302,6 +347,8 @@ ZenEntryPoint::Run()
 				NamedEvent ParentEvent{m_ServerOptions.ChildId};
 				ParentEvent.Set();
 			}
+
+			ReportServiceStatus(ServiceStatus::Running);
 		});
 
 		Server.Run();
@@ -327,6 +374,8 @@ ZenEntryPoint::Run()
 
 	ShutdownServerLogging();
 
+	ReportServiceStatus(ServiceStatus::Stopped);
+
 	return ApplicationExitCode();
 }
 
@@ -396,6 +445,10 @@ main(int argc, char* argv[])
 	signal(SIGINT, utils::SignalCallbackHandler);
 	signal(SIGTERM, utils::SignalCallbackHandler);
 
+#if ZEN_PLATFORM_LINUX
+	IgnoreChildSignals();
+#endif
+
 	try
 	{
 		ZenServerOptions ServerOptions;
diff --git a/src/zenserver/windows/service.cpp b/src/zenserver/windows/service.cpp
index cb87df1f6..4be5e6205 100644
--- a/src/zenserver/windows/service.cpp
+++ b/src/zenserver/windows/service.cpp
@@ -21,7 +21,6 @@ HANDLE				  ghSvcStopEvent = NULL;
 
 void SvcInstall(void);
 
-void ReportSvcStatus(DWORD, DWORD, DWORD);
 void SvcReportEvent(LPTSTR);
 
 WindowsService::WindowsService()
@@ -222,14 +221,7 @@ WindowsService::SvcMain()
 		return 1;
 	}
 
-	// Report running status when initialization is complete.
-
-	ReportSvcStatus(SERVICE_RUNNING, NO_ERROR, 0);
-
 	int ReturnCode = Run();
-
-	ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0);
-
 	return ReturnCode;
 }
 
diff --git a/src/zenserver/windows/service.h b/src/zenserver/windows/service.h
index 7c9610983..ca0270a36 100644
--- a/src/zenserver/windows/service.h
+++ b/src/zenserver/windows/service.h
@@ -2,6 +2,8 @@
 
 #pragma once
 
+#include <zencore/windows.h>
+
 class WindowsService
 {
 public:
@@ -18,3 +20,5 @@ public:
 	int SvcMain();
 	static void __stdcall SvcCtrlHandler(unsigned long);
 };
+
+VOID ReportSvcStatus(DWORD dwCurrentState, DWORD dwWin32ExitCode, DWORD dwWaitHint);
diff --git a/src/zenstore-test/zenstore-test.cpp b/src/zenstore-test/zenstore-test.cpp
index e5b312984..b86f6be15 100644
--- a/src/zenstore-test/zenstore-test.cpp
+++ b/src/zenstore-test/zenstore-test.cpp
@@ -9,6 +9,7 @@
 #if ZEN_WITH_TESTS
 #	define ZEN_TEST_WITH_RUNNER 1
 #	include <zencore/testing.h>
+#	include <zencore/process.h>
 #endif
 
 int
@@ -17,6 +18,10 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 #if ZEN_WITH_TESTS
 	zen::zenstore_forcelinktests();
 
+#if ZEN_PLATFORM_LINUX
+	zen::IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 	zen::MaximizeOpenFileCount();
 
diff --git a/src/zenstore/chunking.cpp b/src/zenstore/chunking.cpp
index 30edd322a..71f0a06e4 100644
--- a/src/zenstore/chunking.cpp
+++ b/src/zenstore/chunking.cpp
@@ -5,6 +5,7 @@
 #include <gsl/gsl-lite.hpp>
 
 #include <cmath>
+#include <cstring>
 
 namespace zen::detail {
 
diff --git a/src/zenutil-test/zenutil-test.cpp b/src/zenutil-test/zenutil-test.cpp
index fadaf0995..a392ab058 100644
--- a/src/zenutil-test/zenutil-test.cpp
+++ b/src/zenutil-test/zenutil-test.cpp
@@ -9,6 +9,7 @@
 #if ZEN_WITH_TESTS
 #	define ZEN_TEST_WITH_RUNNER 1
 #	include <zencore/testing.h>
+#	include <zencore/process.h>
 #endif
 
 int
@@ -17,6 +18,10 @@ main([[maybe_unused]] int argc, [[maybe_unused]] char* argv[])
 #if ZEN_WITH_TESTS
 	zen::zenutil_forcelinktests();
 
+#if ZEN_PLATFORM_LINUX
+	zen::IgnoreChildSignals();
+#endif
+
 	zen::logging::InitializeLogging();
 	zen::MaximizeOpenFileCount();
 
diff --git a/src/zenutil/chunkrequests.cpp b/src/zenutil/chunkrequests.cpp
index 745363668..e28df02a8 100644
--- a/src/zenutil/chunkrequests.cpp
+++ b/src/zenutil/chunkrequests.cpp
@@ -1,5 +1,7 @@
 // Copyright Epic Games, Inc. All Rights Reserved.
 
+#include <algorithm>
+
 #include <zenutil/chunkrequests.h>
 
 #include <zencore/blake3.h>
diff --git a/src/zenutil/include/zenutil/service.h b/src/zenutil/include/zenutil/service.h
index 492e5c80a..2798bcb1f 100644
--- a/src/zenutil/include/zenutil/service.h
+++ b/src/zenutil/include/zenutil/service.h
@@ -20,6 +20,7 @@ struct ServiceSpec
 {
 	std::filesystem::path ExecutablePath;
 	std::string			  CommandLineOptions;
+	std::string			  UserName;
 #if ZEN_PLATFORM_WINDOWS
 	std::string DisplayName;
 	std::string Description;
diff --git a/src/zenutil/service.cpp b/src/zenutil/service.cpp
index 45874d1b5..ea7c2aae6 100644
--- a/src/zenutil/service.cpp
+++ b/src/zenutil/service.cpp
@@ -6,6 +6,7 @@
 #include <zencore/process.h>
 #include <zencore/scopeguard.h>
 #include <zencore/zencore.h>
+#include <string_view>
 
 #if ZEN_PLATFORM_WINDOWS
 #	include <zencore/windows.h>
@@ -23,6 +24,7 @@
 
 #	include <unistd.h>
 #	include <sys/stat.h>
+#	include <regex>
 #endif
 
 namespace zen {
@@ -258,7 +260,6 @@ namespace {
 
 #if ZEN_PLATFORM_MAC || ZEN_PLATFORM_LINUX
 
-	// TODO: Is this good enough to capture all output/errors/return codes?
 	std::pair<int, std::string> ExecuteProgram(std::string_view Cmd)
 	{
 		std::string Data;
@@ -289,10 +290,11 @@ namespace {
 			int Status = pclose(Stream);
 			if (Status < 0)
 			{
+				ZEN_DEBUG("Command {} returned {}, errno {}", Command, Status, errno);
 				return {Status, Data};
 			}
 			uint64_t WaitMS = 100;
-			if (!WIFEXITED(Status))
+			if (WIFEXITED(Status))
 			{
 				Res = WEXITSTATUS(Status);
 			}
@@ -319,10 +321,9 @@ namespace {
 	std::string BuildUnitFile(std::string_view			   ServiceName,
 							  const std::filesystem::path& ExecutablePath,
 							  std::string_view			   CommandLineOptions,
-							  std::string_view			   AliasName)
+							  std::string_view			   AliasName,
+							  std::string_view			   UserName)
 	{
-		// TODO: Revise to make sure the unit file is correct
-		// TODO: Do we need a separate config file or is that optional?
 		return fmt::format(
 			"[Unit]\n"
 			"Description={}\n"
@@ -333,17 +334,17 @@ namespace {
 			"StartLimitIntervalSec=0\n"
 			"\n"
 			"[Service]\n"
-			"Type=simple\n"
+			"Type=notify\n"
 			"Restart=always\n"
 			"RestartSec=1\n"
-			"User=serviceuser\n"
+			"User={}\n"
 			"ExecStart={} {}\n"
-			"Restart=always\n"
 			"RuntimeDirectory={}\n"
 			"[Install]\n"
 			"Alias={}\n"
 			"WantedBy=multi-user.target",
 			ServiceName,
+			UserName,
 			ExecutablePath,
 			CommandLineOptions,
 			ExecutablePath.parent_path(),
@@ -881,12 +882,23 @@ StopService(std::string_view ServiceName)
 std::error_code
 InstallService(std::string_view ServiceName, const ServiceSpec& Spec)
 {
-	// TODO: Do we need to create a separate user for the service or is running as root OK?
-
 	const std::string			UnitName		= GetUnitName(ServiceName);
 	const std::filesystem::path ServiceUnitPath = GetServiceUnitPath(UnitName);
+	std::string UserName = Spec.UserName;
+
+	if (UserName == "")
+	{
+		std::pair<int, std::string> UserResult = ExecuteProgram("echo $SUDO_USER");
+		if (UserResult.first != 0 || UserResult.second.empty())
+		{
+			ZEN_ERROR("Unable to determine current user");	
+			return MakeErrorCode(UserResult.first);
+		}
+
+		UserName = UserResult.second;
+	}
 
-	std::string UnitFile = BuildUnitFile(ServiceName, Spec.ExecutablePath, Spec.CommandLineOptions, UnitName);
+	std::string UnitFile = BuildUnitFile(ServiceName, Spec.ExecutablePath, Spec.CommandLineOptions, UnitName, UserName);
 	ZEN_DEBUG("Writing systemd unit file to {}", ServiceUnitPath.string());
 	try
 	{
@@ -963,17 +975,38 @@ QueryInstalledService(std::string_view ServiceName, ServiceInfo& OutInfo)
 	if (std::filesystem::is_regular_file(ServiceUnitPath))
 	{
 		OutInfo.Status = ServiceStatus::Stopped;
-		// TODO: Read and parse unit file ?
 
-		std::pair<int, std::string> Res = ExecuteProgram(fmt::format("systemctl status {}", UnitName));
+		std::pair<int, std::string> Res = ExecuteProgram(fmt::format("systemctl is-active --quiet {}", UnitName));
 		if (Res.first == 0)
 		{
-			// TODO: What does status really return and what info can we use here to get the ServiceInfo complete?
 			OutInfo.Status = ServiceStatus::Running;
+
+			std::pair<int, std::string> ShowResult = ExecuteProgram(fmt::format("systemctl show -p ExecStart {}", UnitName));
+			if (ShowResult.first == 0)
+			{
+				std::regex Regex(R"~(ExecStart=\{ path=(.*?) ; argv\[\]=(.*?) ;)~");
+				std::smatch Match;
+
+				if (std::regex_search(ShowResult.second, Match, Regex))
+				{
+					std::string Executable = Match[1].str();
+					std::string CommandLine = Match[2].str();
+					OutInfo.Spec.ExecutablePath = Executable;
+					OutInfo.Spec.CommandLineOptions = CommandLine.substr(Executable.size(), CommandLine.size());
+				}
+				else
+				{
+					ZEN_WARN("Failed to parse output of systemctl show: {}", ShowResult.second);
+				}
+			}
+			else
+			{
+				ZEN_WARN("Failed to read start info from systemctl: error code {}", ShowResult.first);
+			}
 		}
 		else
 		{
-			ZEN_DEBUG("systemctl status failed with '{}"({}), Res.second, Res.first);
+			ZEN_DEBUG("systemctl status failed with '{}'({})", Res.second, Res.first);
 		}
 	}
 
diff --git a/src/zenutil/zenserverprocess.cpp b/src/zenutil/zenserverprocess.cpp
index 11fcce02f..0409cb976 100644
--- a/src/zenutil/zenserverprocess.cpp
+++ b/src/zenutil/zenserverprocess.cpp
@@ -168,10 +168,22 @@ ZenServerState::Initialize()
 	}
 #else
 	ZEN_INFO("{}", S_IRUSR | S_IWUSR | S_IXUSR);
+	ZEN_INFO("{}", geteuid());
 	int Fd = shm_open("/UnrealEngineZen", O_RDWR | O_CREAT | O_CLOEXEC, geteuid() == 0 ? 0766 : 0666);
 	if (Fd < 0)
 	{
-		ThrowLastError("Could not open a shared memory object");
+		// Work around a potential issue if the service user is changed in certain configurations.
+		// If the sysctl 'fs.protected_regular' is set to 1 or 2 (default on many distros),
+		// we will be unable to open an existing shared memory object created by another user using O_CREAT,
+		// even if we have the correct permissions, or are running as root. If we destroy the existing
+		// shared memory object and retry, we'll be able to get past shm_open() so long as we have
+		// the appropriate permissions to create the shared memory object.
+		shm_unlink("/UnrealEngineZen");
+		Fd = shm_open("/UnrealEngineZen", O_RDWR | O_CREAT | O_CLOEXEC, geteuid() == 0 ? 0766 : 0666);
+		if (Fd < 0)
+		{
+			ThrowLastError("Could not open a shared memory object");
+		}
 	}
 	fchmod(Fd, 0666);
 	void* hMap = (void*)intptr_t(Fd);