Fixed issues where WaitForMultipleObjects may hang when given a NULL process. Caused by incorrect placement into an array of valid processes.

Fixed bugs in win64 impl where it did not check if the process was a known invalid value

include/uniproc.h

@@ -1,27 +1,33 @@
 #pragma once
+#ifdef __cplusplus
+extern "C"
+{
+#endif
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
 #include <Windows.h>
-typedef struct {
+	typedef struct {
 
-	uint64_t userdata;
-	FILE* in;
-	FILE* out;
-	FILE* err;
+		uint64_t userdata;
+		FILE* in;
+		FILE* out;
+		FILE* err;
 
-	// OS-dependent handle to the process
-	uint32_t _proc_hdl;
-	
-} uniproc_process;
+		// OS-dependent handle to the process
+		uint32_t _proc_hdl;
 
-uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, const char** argv);
-void uniproc_await_processes(const uniproc_process* p, int* return_codes, const size_t num_processes);
-void uniproc_close_process(uniproc_process* p);
-bool uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes);
-void uniproc_nullify_processes(uniproc_process* p, const size_t num_processes);
-bool uniproc_is_process_null(const uniproc_process* p);
-void uniproc_await_any_processes(const uniproc_process* p, const size_t num_processes);
-void uniproc_set_userdata(uniproc_process* p, const uint64_t userdata);
-void uniproc_set_userdata(uniproc_process* p, void const* userdata);
-void* uniproc_get_userdata(uniproc_process* p);
+	} uniproc_process;
+
+	uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, const char** argv);
+	void uniproc_await_processes(const uniproc_process* p, int* return_codes, const size_t num_processes);
+	void uniproc_close_process(uniproc_process* p);
+	int uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes);
+	void uniproc_nullify_processes(uniproc_process* p, const size_t num_processes);
+	int uniproc_is_process_null(const uniproc_process* p);
+	void uniproc_await_any_processes(const uniproc_process* p, const size_t num_processes);
+	void uniproc_set_userdata(uniproc_process* p, void const* userdata);
+	void* uniproc_get_userdata(uniproc_process* p);
+#ifdef __cplusplus
+}
+#endif

src/uniproc_unix.c

@@ -9,6 +9,8 @@
 uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, const char** argv)
 {
 	(void)num_args;
+	uniproc_process p;
+
 	// Create pipes
 	int stdin_pipe[2];
 	int stdout_pipe[2];
@@ -16,22 +18,21 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 
 	if (pipe(stdin_pipe) == -1)
 	{
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (pipe(stdout_pipe) == -1)
 	{
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (pipe(stderr_pipe) == -1)
 	{
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 
 	// Fork process
-	uniproc_process p;
 	p.in = stdin_pipe[1];	// Write handle
 	p.out = stdout_pipe[0];	// Read handle
 	p.err = stderr_pipe[0];	// Read handle
@@ -39,7 +40,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 	pid_t pid = fork();
 	if (pid == -1)
 	{
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	// Child
@@ -48,17 +49,17 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		// Redirect the child's std. files to the pipes created above
 		if (dup2(stdin_pipe[0], STDIN_FILENO) == -1)
 		{
-			uniproc_nullify_processes(p, 1);
+			uniproc_nullify_processes(&p, 1);
 			return p;
 		}
 		if (dup2(stdout_pipe[1], STDOUT_FILENO) == -1)
 		{
-			uniproc_nullify_processes(p, 1);
+			uniproc_nullify_processes(&p, 1);
 			return p;
 		}
 		if (dup2(stdout_pipe[2], STDERR_FILENO) == -1)
 		{
-			uniproc_nullify_processes(p, 1);
+			uniproc_nullify_processes(&p, 1);
 			return p;
 		}
 
@@ -73,7 +74,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		// And execute the command provided
 		if (execvp(cmd, argv) == -1)
 		{
-			uniproc_nullify_processes(p, 1);
+			uniproc_nullify_processes(&p, 1);
 			return p;
 		}
 			
@@ -117,11 +118,12 @@ void uniproc_close_process(uniproc_process* p)
 	fclose(p->in);
 	fclose(p->out);
 	fclose(p->err);
+	uniproc_nullify_processes(p, 1);
 }
 
-bool uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes)
+int uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes)
 {
-	bool res = true;
+	int res = 1;
 
 	for (size_t i = 0; i < num_processes; ++i)
 		res &&= uniproc_is_process_null(p + i) || waitpid(p[i]._proc_hdl, nullptr, WNOHANG | WIFEXITED | WIFSIGNALED | WCOREDUMP);
@@ -132,7 +134,7 @@ bool uniproc_are_processes_finished(const uniproc_process* p, const size_t num_p
 void uniproc_nullify_processes(uniproc_process* p, const size_t num_processes)
 { memset(p, 0, sizeof(uniproc_process) * mum_processes); }
 
-bool uniproc_is_process_null(const uniproc_process* p)
+int uniproc_is_process_null(const uniproc_process* p)
 { return p->_proc_hdl == NULL; }
 
 void uniproc_await_any_processes(const uniproc_process* p, const size_t num_processes)
@@ -141,9 +143,6 @@ void uniproc_await_any_processes(const uniproc_process* p, const size_t num_proc
 		waitpid(0, nullptr, WIFEXITED | WIFSIGNALED | WCOREDUMP);
 }
 
-void uniproc_set_userdata(uniproc_process* p, const uint64_t userdata)
-{ p->userdata = userdata; }
-
 void uniproc_set_userdata(uniproc_process * p, void const* userdata)
 { p->userdata = (uint64_t)userdata; }
 

src/uniproc_win64.c

@@ -6,6 +6,8 @@
 
 uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, const char** argv)
 {
+	uniproc_process p;
+
 	// Create pipes
 	HANDLE child_in;
 	HANDLE in;
@@ -22,14 +24,14 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 	// Actually create the pipes
 	if (!CreatePipe(&out, &child_out, &pipe_attribs, 0))
 	{
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (!CreatePipe(&err, &child_err, &pipe_attribs, 0))
 	{
 		CloseHandle(child_out);
 		CloseHandle(out);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (!CreatePipe(&child_in, &in, &pipe_attribs, 0))
@@ -38,7 +40,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		CloseHandle(out);
 		CloseHandle(child_err);
 		CloseHandle(err);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 
@@ -51,7 +53,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		CloseHandle(out);
 		CloseHandle(child_err);
 		CloseHandle(err);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (!SetHandleInformation(err, HANDLE_FLAG_INHERIT, 0))
@@ -62,7 +64,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		CloseHandle(out);
 		CloseHandle(child_err);
 		CloseHandle(err);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 	if (!SetHandleInformation(in, HANDLE_FLAG_INHERIT, 0))
@@ -73,12 +75,10 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		CloseHandle(out);
 		CloseHandle(child_err);
 		CloseHandle(err);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 
-	// Create child process
-	uniproc_process p;
 
 	// Create p_cmd by appending all argv's to cmd
 	size_t x = strlen(cmd);
@@ -131,7 +131,7 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 		CloseHandle(out);
 		CloseHandle(child_err);
 		CloseHandle(err);
-		uniproc_nullify_processes(p, 1);
+		uniproc_nullify_processes(&p, 1);
 		return p;
 	}
 
@@ -148,53 +148,75 @@ uniproc_process uniproc_create_process(const char* cmd, const size_t num_args, c
 	return p;
 }
 
-void uniproc_await_processes(uniproc_process* p, int* return_codes, size_t num_processes)
+void uniproc_await_processes(const uniproc_process* p, int* return_codes, const size_t num_processes)
 {
+	size_t num_valid_handles = 0;
 	HANDLE* h = (HANDLE*)malloc(num_processes*sizeof(HANDLE));
 	for (size_t i = 0; i < num_processes; ++i)
-		h[i] = (HANDLE)p[i]._proc_hdl;
-
-	WaitForMultipleObjects(num_processes, h, 1, INFINITE);
+	{
+		if (uniproc_is_process_null(p + i)) continue;
+		h[num_valid_handles++] = (HANDLE)p[i]._proc_hdl;
+	}
+	if (num_valid_handles == 0) return;
+	WaitForMultipleObjects(num_valid_handles, h, 1, INFINITE);
 	free(h);
 
 	if (return_codes == NULL) return;
 	for (size_t i = 0; i < num_processes; ++i)
-		GetExitCodeProcess((HANDLE)p[i]._proc_hdl, (LPDWORD)(return_codes+i));
+	{
+		if (uniproc_is_process_null(p + i)) continue;
+		GetExitCodeProcess((HANDLE)p[i]._proc_hdl, (LPDWORD)(return_codes + i));
+	}
 }
 
 void uniproc_close_process(uniproc_process* p)
 {
+	if (uniproc_is_process_null(p)) return;
 	TerminateProcess((HANDLE)p->_proc_hdl, 2);
 	CloseHandle((HANDLE)p->_proc_hdl);
 	fclose(p->in);
 	fclose(p->out);
 	fclose(p->err);
+	uniproc_nullify_processes(p, 1);
 }
 
-bool uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes)
+int uniproc_are_processes_finished(const uniproc_process* p, const size_t num_processes)
 {
 	HANDLE* h = (HANDLE*)malloc(num_processes*sizeof(HANDLE));
-	DWORD ret = WaitForMultipleObjects(num_processes, h, true, 0);
+	size_t num_valid_handles = 0;
+	for (size_t i = 0; i < num_processes; ++i)
+	{
+		if (uniproc_is_process_null(p + i)) continue;
+		h[i] = (HANDLE)p[i]._proc_hdl;
+		num_valid_handles++;
+	}
+	if (num_valid_handles == 0) return 1;
+	DWORD ret = WaitForMultipleObjects(num_valid_handles, h, 1, 0);
 	free(h);
-	return ret >= WAIT_OBJECT_0 && ret < WAIT_OBJECT_0 + num_processes;
+	return ret >= WAIT_OBJECT_0 && ret < WAIT_OBJECT_0 + num_valid_handles;
 }
 
 void uniproc_nullify_processes(uniproc_process* p, const size_t num_processes)
 { memset(p, 0, num_processes * sizeof(uniproc_process)); }
 
-void uniproc_is_process_null(const uniproc_process* p)
+int uniproc_is_process_null(const uniproc_process* p)
 { return p->_proc_hdl == NULL; }
 
 void uniproc_await_any_processes(const uniproc_process* p, const size_t num_processes)
 {
 	HANDLE* h = (HANDLE*)malloc(num_processes * sizeof(HANDLE));
-	WaitForMultipleObjects(num_processes, h, false, INFINITE);
+	size_t num_valid_handles = 0;
+	for (size_t i = 0; i < num_processes; ++i)
+	{
+		if (uniproc_is_process_null(p + i)) continue;
+		h[i] = (HANDLE)p[i]._proc_hdl;
+		num_valid_handles++;
+	}
+	if (num_valid_handles == 0) return;
+	WaitForMultipleObjects(num_valid_handles, h, 0, INFINITE);
 	free(h);
 }
 
-void uniproc_set_userdata(uniproc_process* p, const uint64_t userdata)
-{ p->userdata = userdata; }
-
 void uniproc_set_userdata(uniproc_process * p, void const* userdata)
 { p->userdata = (uint64_t)userdata; }