Merge pull request #90358 from KoBeWi/finding_errors_in_other_apps

Add `OS.get_process_exit_code()` method
godotengine · Apr 16, 2024 · 7210d6c · 7210d6c
2 parents c2d2ff6 + dce4a3e
commit 7210d6c
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 3 deletions.
diff --git a/core/core_bind.cpp b/core/core_bind.cpp
@@ -338,6 +338,10 @@ bool OS::is_process_running(int p_pid) const {
 	return ::OS::get_singleton()->is_process_running(p_pid);
 }
 
+int OS::get_process_exit_code(int p_pid) const {
+	return ::OS::get_singleton()->get_process_exit_code(p_pid);
+}
+
 int OS::get_process_id() const {
 	return ::OS::get_singleton()->get_process_id();
 }
@@ -602,6 +606,7 @@ void OS::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("shell_open", "uri"), &OS::shell_open);
 	ClassDB::bind_method(D_METHOD("shell_show_in_file_manager", "file_or_dir_path", "open_folder"), &OS::shell_show_in_file_manager, DEFVAL(true));
 	ClassDB::bind_method(D_METHOD("is_process_running", "pid"), &OS::is_process_running);
+	ClassDB::bind_method(D_METHOD("get_process_exit_code", "pid"), &OS::get_process_exit_code);
 	ClassDB::bind_method(D_METHOD("get_process_id"), &OS::get_process_id);
 
 	ClassDB::bind_method(D_METHOD("has_environment", "variable"), &OS::has_environment);

diff --git a/core/core_bind.h b/core/core_bind.h
@@ -164,6 +164,7 @@ class OS : public Object {
 	Error shell_show_in_file_manager(const String &p_path, bool p_open_folder = true);
 
 	bool is_process_running(int p_pid) const;
+	int get_process_exit_code(int p_pid) const;
 	int get_process_id() const;
 
 	void set_restart_on_exit(bool p_restart, const Vector<String> &p_restart_arguments = Vector<String>());

diff --git a/core/os/os.h b/core/os/os.h
@@ -176,6 +176,7 @@ class OS {
 	virtual Error kill(const ProcessID &p_pid) = 0;
 	virtual int get_process_id() const;
 	virtual bool is_process_running(const ProcessID &p_pid) const = 0;
+	virtual int get_process_exit_code(const ProcessID &p_pid) const = 0;
 	virtual void vibrate_handheld(int p_duration_ms = 500) {}
 
 	virtual Error shell_open(const String &p_uri);

diff --git a/doc/classes/OS.xml b/doc/classes/OS.xml
@@ -408,11 +408,20 @@
 				[b]Note:[/b] On Web platforms, it is still possible to determine the host platform's OS with feature tags. See [method has_feature].
 			</description>
 		</method>
+		<method name="get_process_exit_code" qualifiers="const">
+			<return type="int" />
+			<param index="0" name="pid" type="int" />
+			<description>
+				Returns the exit code of a spawned process once it has finished running (see [method is_process_running]).
+				Returns [code]-1[/code] if the [param pid] is not a PID of a spawned child process, the process is still running, or the method is not implemented for the current platform.
+				[b]Note:[/b] This method is implemented on Android, Linux, macOS and Windows.
+			</description>
+		</method>
 		<method name="get_process_id" qualifiers="const">
 			<return type="int" />
 			<description>
 				Returns the number used by the host machine to uniquely identify this application.
-				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS and Windows.
+				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS, and Windows.
 			</description>
 		</method>
 		<method name="get_processor_count" qualifiers="const">
@@ -592,7 +601,7 @@
 			<param index="0" name="pid" type="int" />
 			<description>
 				Returns [code]true[/code] if the child process ID ([param pid]) is still running or [code]false[/code] if it has terminated. [param pid] must be a valid ID generated from [method create_process].
-				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS and Windows.
+				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS, and Windows.
 			</description>
 		</method>
 		<method name="is_restart_on_exit_set" qualifiers="const">

diff --git a/drivers/unix/os_unix.cpp b/drivers/unix/os_unix.cpp
@@ -168,11 +168,13 @@ void OS_Unix::initialize_core() {
 
 	NetSocketPosix::make_default();
 	IPUnix::make_default();
+	process_map = memnew((HashMap<ProcessID, ProcessInfo>));
 
 	_setup_clock();
 }
 
 void OS_Unix::finalize_core() {
+	memdelete(process_map);
 	NetSocketPosix::cleanup();
 }
 
@@ -582,6 +584,11 @@ Dictionary OS_Unix::execute_with_pipe(const String &p_path, const List<String> &
 	err_pipe.instantiate();
 	err_pipe->open_existing(pipe_err[0], 0);
 
+	ProcessInfo pi;
+	process_map_mutex.lock();
+	process_map->insert(pid, pi);
+	process_map_mutex.unlock();
+
 	ret["stdio"] = main_pipe;
 	ret["stderr"] = err_pipe;
 	ret["pid"] = pid;
@@ -698,6 +705,11 @@ Error OS_Unix::create_process(const String &p_path, const List<String> &p_argume
 		raise(SIGKILL);
 	}
 
+	ProcessInfo pi;
+	process_map_mutex.lock();
+	process_map->insert(pid, pi);
+	process_map_mutex.unlock();
+
 	if (r_child_id) {
 		*r_child_id = pid;
 	}
@@ -720,14 +732,45 @@ int OS_Unix::get_process_id() const {
 }
 
 bool OS_Unix::is_process_running(const ProcessID &p_pid) const {
+	MutexLock lock(process_map_mutex);
+	const ProcessInfo *pi = process_map->getptr(p_pid);
+
+	if (pi && !pi->is_running) {
+		return false;
+	}
+
 	int status = 0;
 	if (waitpid(p_pid, &status, WNOHANG) != 0) {
+		if (pi) {
+			pi->is_running = false;
+			pi->exit_code = status;
+		}
 		return false;
 	}
 
 	return true;
 }
 
+int OS_Unix::get_process_exit_code(const ProcessID &p_pid) const {
+	MutexLock lock(process_map_mutex);
+	const ProcessInfo *pi = process_map->getptr(p_pid);
+
+	if (pi && !pi->is_running) {
+		return pi->exit_code;
+	}
+
+	int status = 0;
+	if (waitpid(p_pid, &status, WNOHANG) != 0) {
+		status = WIFEXITED(status) ? WEXITSTATUS(status) : status;
+		if (pi) {
+			pi->is_running = false;
+			pi->exit_code = status;
+		}
+		return status;
+	}
+	return -1;
+}
+
 String OS_Unix::get_locale() const {
 	if (!has_environment("LANG")) {
 		return "en";

diff --git a/drivers/unix/os_unix.h b/drivers/unix/os_unix.h
@@ -37,6 +37,13 @@
 #include "drivers/unix/ip_unix.h"
 
 class OS_Unix : public OS {
+	struct ProcessInfo {
+		mutable bool is_running = true;
+		mutable int exit_code = -1;
+	};
+	HashMap<ProcessID, ProcessInfo> *process_map = nullptr;
+	Mutex process_map_mutex;
+
 protected:
 	// UNIX only handles the core functions.
 	// inheriting platforms under unix (eg. X11) should handle the rest
@@ -81,6 +88,7 @@ class OS_Unix : public OS {
 	virtual Error kill(const ProcessID &p_pid) override;
 	virtual int get_process_id() const override;
 	virtual bool is_process_running(const ProcessID &p_pid) const override;
+	virtual int get_process_exit_code(const ProcessID &p_pid) const override;
 
 	virtual bool has_environment(const String &p_var) const override;
 	virtual String get_environment(const String &p_var) const override;

diff --git a/platform/web/os_web.cpp b/platform/web/os_web.cpp
@@ -132,6 +132,10 @@ bool OS_Web::is_process_running(const ProcessID &p_pid) const {
 	return false;
 }
 
+int OS_Web::get_process_exit_code(const ProcessID &p_pid) const {
+	return -1;
+}
+
 int OS_Web::get_processor_count() const {
 	return godot_js_os_hw_concurrency_get();
 }

diff --git a/platform/web/os_web.h b/platform/web/os_web.h
@@ -85,6 +85,7 @@ class OS_Web : public OS_Unix {
 	Error kill(const ProcessID &p_pid) override;
 	int get_process_id() const override;
 	bool is_process_running(const ProcessID &p_pid) const override;
+	int get_process_exit_code(const ProcessID &p_pid) const override;
 	int get_processor_count() const override;
 	String get_unique_id() const override;
 	int get_default_thread_pool_size() const override { return 1; }

diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp
@@ -867,7 +867,9 @@ Dictionary OS_Windows::execute_with_pipe(const String &p_path, const List<String
 	CloseHandle(pipe_err[1]);
 
 	ProcessID pid = pi.pi.dwProcessId;
+	process_map_mutex.lock();
 	process_map->insert(pid, pi);
+	process_map_mutex.unlock();
 
 	Ref<FileAccessWindowsPipe> main_pipe;
 	main_pipe.instantiate();
@@ -1014,13 +1016,16 @@ Error OS_Windows::create_process(const String &p_path, const List<String> &p_arg
 	if (r_child_id) {
 		*r_child_id = pid;
 	}
+	process_map_mutex.lock();
 	process_map->insert(pid, pi);
+	process_map_mutex.unlock();
 
 	return OK;
 }
 
 Error OS_Windows::kill(const ProcessID &p_pid) {
 	int ret = 0;
+	MutexLock lock(process_map_mutex);
 	if (process_map->has(p_pid)) {
 		const PROCESS_INFORMATION pi = (*process_map)[p_pid].pi;
 		process_map->erase(p_pid);
@@ -1046,24 +1051,58 @@ int OS_Windows::get_process_id() const {
 }
 
 bool OS_Windows::is_process_running(const ProcessID &p_pid) const {
+	MutexLock lock(process_map_mutex);
 	if (!process_map->has(p_pid)) {
 		return false;
 	}
 
-	const PROCESS_INFORMATION &pi = (*process_map)[p_pid].pi;
+	const ProcessInfo &info = (*process_map)[p_pid];
+	if (!info.is_running) {
+		return false;
+	}
 
+	const PROCESS_INFORMATION &pi = info.pi;
 	DWORD dw_exit_code = 0;
 	if (!GetExitCodeProcess(pi.hProcess, &dw_exit_code)) {
 		return false;
 	}
 
 	if (dw_exit_code != STILL_ACTIVE) {
+		info.is_running = false;
+		info.exit_code = dw_exit_code;
 		return false;
 	}
 
 	return true;
 }
 
+int OS_Windows::get_process_exit_code(const ProcessID &p_pid) const {
+	MutexLock lock(process_map_mutex);
+	if (!process_map->has(p_pid)) {
+		return -1;
+	}
+
+	const ProcessInfo &info = (*process_map)[p_pid];
+	if (!info.is_running) {
+		return info.exit_code;
+	}
+
+	const PROCESS_INFORMATION &pi = info.pi;
+
+	DWORD dw_exit_code = 0;
+	if (!GetExitCodeProcess(pi.hProcess, &dw_exit_code)) {
+		return -1;
+	}
+
+	if (dw_exit_code == STILL_ACTIVE) {
+		return -1;
+	}
+
+	info.is_running = false;
+	info.exit_code = dw_exit_code;
+	return dw_exit_code;
+}
+
 Error OS_Windows::set_cwd(const String &p_cwd) {
 	if (_wchdir((LPCWSTR)(p_cwd.utf16().get_data())) != 0) {
 		return ERR_CANT_OPEN;

diff --git a/platform/windows/os_windows.h b/platform/windows/os_windows.h
@@ -150,8 +150,11 @@ class OS_Windows : public OS {
 	struct ProcessInfo {
 		STARTUPINFO si;
 		PROCESS_INFORMATION pi;
+		mutable bool is_running = true;
+		mutable int exit_code = -1;
 	};
 	HashMap<ProcessID, ProcessInfo> *process_map = nullptr;
+	Mutex process_map_mutex;
 
 public:
 	virtual void alert(const String &p_alert, const String &p_title = "ALERT!") override;
@@ -189,6 +192,7 @@ class OS_Windows : public OS {
 	virtual Error kill(const ProcessID &p_pid) override;
 	virtual int get_process_id() const override;
 	virtual bool is_process_running(const ProcessID &p_pid) const override;
+	virtual int get_process_exit_code(const ProcessID &p_pid) const override;
 
 	virtual bool has_environment(const String &p_var) const override;
 	virtual String get_environment(const String &p_var) const override;