added `--detach` option to zenserver (#115)

added `--detach` option to zenserver. When this is passed in with a false value, we do not create a new process group in order to behave more as expected when running with `xmake run zenserver`. Without this change the zenserver process does not receive any signals and won't exit when xmake does, causing processes to linger in the background. 

The default behaviour (when run from UE) is unchanged.

5 files changed, 42 insertions, 5 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d56570520..e48b34f38 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,12 +4,14 @@
 - Feature: Added new options to `zen gc` command for GC V2
   - `--single-threaded` GCV2 - force GC operation to run in single-threaded mode to help with debugging, default is off
 - Feature: Project store now compacts oplog container at first open when 50% of storage is unused and during GCv2 when more than 25% is unused
+- Feature: Added `--detach` option to zenserver which controls the call to `setsid()` on Mac/Linux for better ctrl-c behaviour when running it using `xmake run`
 - Bugfix: Absolute paths and paths going outside the root path for workspace shares are now blocked
 - Bugfix: Fix ASSERT that would trigger in GC under certain conditions if source block was empty
 - Bugfix: Skip and report invalid configurations for workspaces instead of crashing
 - Bugfix: Report back error to http caller if removal of oplog fails
 - Bugfix: If a cache bucket value fails validation - don't try to memcache the empty buffer
 - Bugfix: Skip chunk in block stores when iterating a block if the location is out of range
+- Improvement: `xmake run zenserver ...` now passes in `--detach=false` to allow subprocesses to be terminated using Ctrl-C (previously, zenserver instances would linger)
 - Improvement: `zen workspace-share create` now resolves relative root paths to absolute paths
 - Improvement: Add better output/logging when failing to initialize shared mutex
 - Improvement: Validate data when gathering attachments in GCv2
diff --git a/src/zenserver/config.cpp b/src/zenserver/config.cpp
index 867089ded..b430db629 100644
--- a/src/zenserver/config.cpp
+++ b/src/zenserver/config.cpp
@@ -611,6 +611,9 @@ ParseCliOptions(int argc, char* argv[], ZenServerOptions& ServerOptions)
 	options.add_options()("sentry-allow-personal-info",
 						  "Allow personally identifiable information in sentry crash reports",
 						  cxxopts::value<bool>(ServerOptions.SentryAllowPII)->default_value("false"));
+	options.add_options()("detach",
+						  "Indicate whether zenserver should detach from parent process group",
+						  cxxopts::value<bool>(ServerOptions.Detach)->default_value("true"));
 
 	// clang-format off
 	options.add_options("logging")
diff --git a/src/zenserver/config.h b/src/zenserver/config.h
index 46fdd6fdf..4c9b9361c 100644
--- a/src/zenserver/config.h
+++ b/src/zenserver/config.h
@@ -157,6 +157,7 @@ struct ZenServerOptions
 	bool					 IsFirstRun			= false;
 	bool					 NoSentry			= false;
 	bool					 SentryAllowPII		= false;  // Allow personally identifiable information in sentry crash reports
+	bool					 Detach				= true;	  // Whether zenserver should detach from existing process group (Mac/Linux)
 	bool					 ObjectStoreEnabled = false;
 	bool					 NoConsoleOutput	= false;  // Control default use of stdout for diagnostics
 	std::string				 Loggers[zen::logging::level::LogLevelCount];
diff --git a/src/zenserver/main.cpp b/src/zenserver/main.cpp
index c27d4eb04..e6ecc3ce2 100644
--- a/src/zenserver/main.cpp
+++ b/src/zenserver/main.cpp
@@ -351,11 +351,6 @@ main(int argc, char* argv[])
 		}
 	}
 
-#if ZEN_PLATFORM_LINUX | ZEN_PLATFORM_MAC
-	// Detach ourselves from any parent process
-	setsid();
-#endif
-
 	signal(SIGINT, utils::SignalCallbackHandler);
 	signal(SIGTERM, utils::SignalCallbackHandler);
 
@@ -364,6 +359,14 @@ main(int argc, char* argv[])
 		ZenServerOptions ServerOptions;
 		ParseCliOptions(argc, argv, ServerOptions);
 
+		if (ServerOptions.Detach)
+		{
+#if ZEN_PLATFORM_LINUX | ZEN_PLATFORM_MAC
+			// Detach ourselves from any parent process
+			setsid();
+#endif
+		}
+
 		std::string_view DeleteReason;
 
 		if (ServerOptions.IsCleanStart)
diff --git a/src/zenserver/xmake.lua b/src/zenserver/xmake.lua
index c42f305ee..a3d7aa124 100644
--- a/src/zenserver/xmake.lua
+++ b/src/zenserver/xmake.lua
@@ -68,3 +68,31 @@ target("zenserver")
         -- line below forces breakpad_client to be to the right of sentry_native
         add_syslinks("breakpad_client")
     end
+
+    -- to work around some unfortunate Ctrl-C behaviour on Linux/Mac due to 
+    -- our use of setsid() at startup we pass in `--no-detach` to zenserver
+    -- ensure that it recieves signals when the user requests termination
+    on_run(function(target)
+        -- the following is roughly cribbed from xmake/actions/run/xmake.lua
+        -- it would be nicer if we had the option of amending the arguments
+        -- via before_run for instance, but I can't figure out a way to do that
+        import("core.base.option")
+
+        -- get the run directory of target
+        local rundir = target:rundir()
+
+        -- get the absolute target file path
+        local targetfile = path.absolute(target:targetfile())
+
+        -- get run arguments
+        local args = table.wrap(option.get("arguments") or target:get("runargs"))
+
+        table.insert(args, "--detach=false")
+
+        -- debugging?
+        if option.get("debug") then
+            debugger.run(targetfile, args, {curdir = rundir})
+        else
+            os.execv(targetfile, args, {curdir = rundir, detach = option.get("detach")})
+        end
+    end)