<EC>: perhaps fixed race conditions and problems when creating and terminating process groups on Win32

* Sometimes already the job assignment failed in CI builds.
  We now check whether the process is still alive before throwing an error.
* We now set the JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE flag.
  This theoretically shouldn't be necessary when using TerminateJobObject(),
  but who knows.

2 files changed, 34 insertions, 4 deletions

diff --git a/src/error.h b/src/error.h
index 469d957..5ffd434 100644
--- a/src/error.h
+++ b/src/error.h
@@ -50,6 +50,7 @@ typedef enum {
 	TECO_ERROR_EDITINGLOCALQREG,
 	TECO_ERROR_MEMLIMIT,
 	TECO_ERROR_CLIPBOARD,
+	TECO_ERROR_WIN32,
 
 	/** Interrupt current operation */
 	TECO_ERROR_INTERRUPTED,
@@ -138,6 +139,15 @@ teco_error_editinglocalqreg_set(GError **error, const gchar *name, gsize len)
 	            "Editing local Q-Register \"%s\" at end of macro call", name_printable);
 }
 
+#ifdef G_OS_WIN32
+static inline void
+teco_error_win32_set(GError **error, const gchar *prefix, gint err)
+{
+	g_autofree gchar *msg = g_win32_error_message(err);
+	g_set_error(error, TECO_ERROR, TECO_ERROR_WIN32, "%s: %s", prefix, msg);
+}
+#endif
+
 static inline void
 teco_error_interrupted_set(GError **error)
 {
diff --git a/src/spawn.c b/src/spawn.c
index e6d620c..a1b8ac8 100644
--- a/src/spawn.c
+++ b/src/spawn.c
@@ -294,15 +294,35 @@ teco_state_execute_done(teco_machine_main_t *ctx, const teco_string_t *str, GErr
 	teco_spawn_ctx.interrupted = FALSE;
 
 #ifdef G_OS_WIN32
+	teco_spawn_ctx.pid = CreateJobObject(NULL, NULL);
+	if (!teco_spawn_ctx.pid) {
+		teco_error_win32_set(error, "Cannot create job object", GetLastError());
+		goto gerror;
+	}
+	JOBOBJECT_EXTENDED_LIMIT_INFORMATION job_info = {
+		.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
+	};
+	if (!SetInformationJobObject(teco_spawn_ctx.pid, JobObjectExtendedLimitInformation,
+	                             &job_info, sizeof(job_info))) {
+		CloseHandle(teco_spawn_ctx.pid);
+		teco_error_win32_set(error, "Cannot configure job object", GetLastError());
+		goto gerror;
+	}
 	/*
 	 * Assigning the process to a job object will allow us to
 	 * kill the entire process tree relatively easily and without
 	 * race conditions.
+	 * There can however be a race condition while assigning the
+	 * job object since the process could already be dead.
 	 */
-	teco_spawn_ctx.pid = CreateJobObjectA(NULL, NULL);
-	if (!teco_spawn_ctx.pid || !AssignProcessToJobObject(teco_spawn_ctx.pid, pid)) {
-		g_set_error(error, TECO_ERROR, TECO_ERROR_FAILED,
-		            "Cannot assign process to job object (%lu)", GetLastError());
+	DWORD exit_code;
+	if (!AssignProcessToJobObject(teco_spawn_ctx.pid, pid) &&
+	    (GetLastError() != ERROR_ACCESS_DENIED ||
+	     !GetExitCodeProcess(teco_spawn_ctx.pid, &exit_code) ||
+	     exit_code == STILL_ACTIVE)) {
+		CloseHandle(teco_spawn_ctx.pid);
+		teco_error_win32_set(error, "Cannot assign process to job object",
+		                     GetLastError());
 		goto gerror;
 	}