Browse Source

Merge pull request #44514 from madmiraal/split-os-execute

Split OS::execute into two methods
Rémi Verschelde 4 years ago
parent
commit
1218441b16

+ 20 - 11
core/core_bind.cpp

@@ -228,24 +228,32 @@ Error _OS::shell_open(String p_uri) {
 	return OS::get_singleton()->shell_open(p_uri);
 }
 
-int _OS::execute(const String &p_path, const Vector<String> &p_arguments, bool p_blocking, Array p_output, bool p_read_stderr) {
-	OS::ProcessID pid = -2;
-	int exitcode = 0;
+int _OS::execute(const String &p_path, const Vector<String> &p_arguments, Array r_output, bool p_read_stderr) {
 	List<String> args;
 	for (int i = 0; i < p_arguments.size(); i++) {
 		args.push_back(p_arguments[i]);
 	}
 	String pipe;
-	Error err = OS::get_singleton()->execute(p_path, args, p_blocking, &pid, &pipe, &exitcode, p_read_stderr);
-	p_output.clear();
-	p_output.push_back(pipe);
+	int exitcode = 0;
+	Error err = OS::get_singleton()->execute(p_path, args, &pipe, &exitcode, p_read_stderr);
+	r_output.push_back(pipe);
+	if (err != OK) {
+		return -1;
+	}
+	return exitcode;
+}
+
+int _OS::create_process(const String &p_path, const Vector<String> &p_arguments) {
+	List<String> args;
+	for (int i = 0; i < p_arguments.size(); i++) {
+		args.push_back(p_arguments[i]);
+	}
+	OS::ProcessID pid = 0;
+	Error err = OS::get_singleton()->create_process(p_path, args, &pid);
 	if (err != OK) {
 		return -1;
-	} else if (p_blocking) {
-		return exitcode;
-	} else {
-		return pid;
 	}
+	return pid;
 }
 
 Error _OS::kill(int p_pid) {
@@ -697,7 +705,8 @@ void _OS::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_processor_count"), &_OS::get_processor_count);
 
 	ClassDB::bind_method(D_METHOD("get_executable_path"), &_OS::get_executable_path);
-	ClassDB::bind_method(D_METHOD("execute", "path", "arguments", "blocking", "output", "read_stderr"), &_OS::execute, DEFVAL(true), DEFVAL(Array()), DEFVAL(false));
+	ClassDB::bind_method(D_METHOD("execute", "path", "arguments", "output", "read_stderr"), &_OS::execute, DEFVAL(Array()), DEFVAL(false));
+	ClassDB::bind_method(D_METHOD("create_process", "path", "arguments"), &_OS::create_process);
 	ClassDB::bind_method(D_METHOD("kill", "pid"), &_OS::kill);
 	ClassDB::bind_method(D_METHOD("shell_open", "uri"), &_OS::shell_open);
 	ClassDB::bind_method(D_METHOD("get_process_id"), &_OS::get_process_id);

+ 2 - 2
core/core_bind.h

@@ -155,8 +155,8 @@ public:
 	int get_low_processor_usage_mode_sleep_usec() const;
 
 	String get_executable_path() const;
-	int execute(const String &p_path, const Vector<String> &p_arguments, bool p_blocking = true, Array p_output = Array(), bool p_read_stderr = false);
-
+	int execute(const String &p_path, const Vector<String> &p_arguments, Array r_output = Array(), bool p_read_stderr = false);
+	int create_process(const String &p_path, const Vector<String> &p_arguments);
 	Error kill(int p_pid);
 	Error shell_open(String p_uri);
 

+ 2 - 1
core/os/os.h

@@ -129,7 +129,8 @@ public:
 	virtual int get_low_processor_usage_mode_sleep_usec() const;
 
 	virtual String get_executable_path() const;
-	virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) = 0;
+	virtual Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) = 0;
+	virtual Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr) = 0;
 	virtual Error kill(const ProcessID &p_pid) = 0;
 	virtual int get_process_id() const;
 	virtual void vibrate_handheld(int p_duration_ms = 500);

+ 34 - 25
doc/classes/OS.xml

@@ -25,6 +25,29 @@
 				[b]Note:[/b] This method is implemented on Linux, macOS and Windows.
 			</description>
 		</method>
+		<method name="create_process">
+			<return type="int">
+			</return>
+			<argument index="0" name="path" type="String">
+			</argument>
+			<argument index="1" name="arguments" type="PackedStringArray">
+			</argument>
+			<description>
+				Creates a new process that runs independently of Godot. It will not terminate if Godot terminates. The file specified in [code]path[/code] must exist and be executable. Platform path resolution will be used. The [code]arguments[/code] are used in the given order and separated by a space.
+				If the process creation succeeds, the method will return the new process ID, which you can use to monitor the process (and potentially terminate it with [method kill]). If the process creation fails, the method will return [code]-1[/code].
+				For example, running another instance of the project:
+				[codeblocks]
+				[gdscript]
+				var pid = OS.create_process(OS.get_executable_path(), [])
+				[/gdscript]
+				[csharp]
+				var pid = OS.CreateProcess(OS.GetExecutablePath(), new string[] {});
+				[/csharp]
+				[/codeblocks]
+				See [method execute] if you wish to run an external command and retrieve the results.
+				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS and Windows.
+			</description>
+		</method>
 		<method name="delay_msec" qualifiers="const">
 			<return type="void">
 			</return>
@@ -71,48 +94,34 @@
 			</argument>
 			<argument index="1" name="arguments" type="PackedStringArray">
 			</argument>
-			<argument index="2" name="blocking" type="bool" default="true">
-			</argument>
-			<argument index="3" name="output" type="Array" default="[  ]">
+			<argument index="2" name="output" type="Array" default="[  ]">
 			</argument>
-			<argument index="4" name="read_stderr" type="bool" default="false">
+			<argument index="3" name="read_stderr" type="bool" default="false">
 			</argument>
 			<description>
-				Execute the file at the given path with the arguments passed as an array of strings. Platform path resolution will take place. The resolved file must exist and be executable.
-				The arguments are used in the given order and separated by a space, so [code]OS.execute("ping", ["-w", "3", "godotengine.org"], false)[/code] will resolve to [code]ping -w 3 godotengine.org[/code] in the system's shell.
-				This method has slightly different behavior based on whether the [code]blocking[/code] mode is enabled.
-				If [code]blocking[/code] is [code]true[/code], the Godot thread will pause its execution while waiting for the process to terminate. The shell output of the process will be written to the [code]output[/code] array as a single string. When the process terminates, the Godot thread will resume execution.
-				If [code]blocking[/code] is [code]false[/code], the Godot thread will continue while the new process runs. It is not possible to retrieve the shell output in non-blocking mode, so [code]output[/code] will be empty.
-				The return value also depends on the blocking mode. When blocking, the method will return an exit code of the process. When non-blocking, the method returns a process ID, which you can use to monitor the process (and potentially terminate it with [method kill]). If the process forking (non-blocking) or opening (blocking) fails, the method will return [code]-1[/code] or another exit code.
-				Example of blocking mode and retrieving the shell output:
+				Executes a command. The file specified in [code]path[/code] must exist and be executable. Platform path resolution will be used. The [code]arguments[/code] are used in the given order and separated by a space. If an [code]output[/code] [Array] is provided, the complete shell output of the process will be appended as a single [String] element in [code]output[/code]. If [code]read_stderr[/code] is [code]true[/code], the output to the standard error stream will be included too.
+				If the command is successfully executed, the method will return the exit code of the command, or [code]-1[/code] if it fails.
+				[b]Note:[/b] The Godot thread will pause its execution until the executed command terminates. Use [Thread] to create a separate thread that will not pause the Godot thread, or use [method create_process] to create a completely independent process.
+				For example, to retrieve a list of the working directory's contents:
 				[codeblocks]
 				[gdscript]
 				var output = []
-				var exit_code = OS.execute("ls", ["-l", "/tmp"], true, output)
+				var exit_code = OS.execute("ls", ["-l", "/tmp"], output)
 				[/gdscript]
 				[csharp]
 				var output = new Godot.Collections.Array();
-				int exitCode = OS.Execute("ls", new string[] {"-l", "/tmp"}, true, output);
-				[/csharp]
-				[/codeblocks]
-				Example of non-blocking mode, running another instance of the project and storing its process ID:
-				[codeblocks]
-				[gdscript]
-				var pid = OS.execute(OS.get_executable_path(), [], false)
-				[/gdscript]
-				[csharp]
-				var pid = OS.Execute(OS.GetExecutablePath(), new string[] {}, false);
+				int exitCode = OS.Execute("ls", new string[] {"-l", "/tmp"}, output);
 				[/csharp]
 				[/codeblocks]
-				If you wish to access a shell built-in or perform a composite command, a platform-specific shell can be invoked. For example:
+				To execute a composite command, a platform-specific shell can be invoked. For example:
 				[codeblocks]
 				[gdscript]
 				var output = []
-				OS.execute("CMD.exe", ["/C", "cd %TEMP% &amp;&amp; dir"], true, output)
+				OS.execute("CMD.exe", ["/C", "cd %TEMP% &amp;&amp; dir"], output)
 				[/gdscript]
 				[csharp]
 				var output = new Godot.Collections.Array();
-				OS.Execute("CMD.exe", new string[] {"/C", "cd %TEMP% &amp;&amp; dir"}, true, output);
+				OS.Execute("CMD.exe", new string[] {"/C", "cd %TEMP% &amp;&amp; dir"}, output);
 				[/csharp]
 				[/codeblocks]
 				[b]Note:[/b] This method is implemented on Android, iOS, Linux, macOS and Windows.

+ 50 - 42
drivers/unix/os_unix.cpp

@@ -257,31 +257,26 @@ uint64_t OS_Unix::get_ticks_usec() const {
 	return longtime;
 }
 
-Error OS_Unix::execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+Error OS_Unix::execute(const String &p_path, const List<String> &p_arguments, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
 #ifdef __EMSCRIPTEN__
 	// Don't compile this code at all to avoid undefined references.
 	// Actual virtual call goes to OS_JavaScript.
 	ERR_FAIL_V(ERR_BUG);
 #else
-	if (p_blocking && r_pipe) {
-		String argss;
-		argss = "\"" + p_path + "\"";
-
+	if (r_pipe) {
+		String command = "\"" + p_path + "\"";
 		for (int i = 0; i < p_arguments.size(); i++) {
-			argss += String(" \"") + p_arguments[i] + "\"";
+			command += String(" \"") + p_arguments[i] + "\"";
 		}
-
 		if (read_stderr) {
-			argss += " 2>&1"; // Read stderr too
+			command += " 2>&1"; // Include stderr
 		} else {
-			argss += " 2>/dev/null"; //silence stderr
+			command += " 2>/dev/null"; // Silence stderr
 		}
-		FILE *f = popen(argss.utf8().get_data(), "r");
-
-		ERR_FAIL_COND_V_MSG(!f, ERR_CANT_OPEN, "Cannot pipe stream from process running with following arguments '" + argss + "'.");
 
+		FILE *f = popen(command.utf8().get_data(), "r");
+		ERR_FAIL_COND_V_MSG(!f, ERR_CANT_OPEN, "Cannot create pipe from command: " + command);
 		char buf[65535];
-
 		while (fgets(buf, 65535, f)) {
 			if (p_pipe_mutex) {
 				p_pipe_mutex->lock();
@@ -292,10 +287,10 @@ Error OS_Unix::execute(const String &p_path, const List<String> &p_arguments, bo
 			}
 		}
 		int rv = pclose(f);
+
 		if (r_exitcode) {
 			*r_exitcode = WEXITSTATUS(rv);
 		}
-
 		return OK;
 	}
 
@@ -303,45 +298,58 @@ Error OS_Unix::execute(const String &p_path, const List<String> &p_arguments, bo
 	ERR_FAIL_COND_V(pid < 0, ERR_CANT_FORK);
 
 	if (pid == 0) {
-		// is child
-
-		if (!p_blocking) {
-			// For non blocking calls, create a new session-ID so parent won't wait for it.
-			// This ensures the process won't go zombie at end.
-			setsid();
-		}
-
-		Vector<CharString> cs;
-		cs.push_back(p_path.utf8());
-		for (int i = 0; i < p_arguments.size(); i++) {
-			cs.push_back(p_arguments[i].utf8());
-		}
-
+		// The child process
 		Vector<char *> args;
-		for (int i = 0; i < cs.size(); i++) {
-			args.push_back((char *)cs[i].get_data());
+		args.push_back((char *)p_path.utf8().get_data());
+		for (int i = 0; i < p_arguments.size(); i++) {
+			args.push_back((char *)p_arguments[i].utf8().get_data());
 		}
 		args.push_back(0);
 
 		execvp(p_path.utf8().get_data(), &args[0]);
-		// still alive? something failed..
-		fprintf(stderr, "**ERROR** OS_Unix::execute - Could not create child process while executing: %s\n", p_path.utf8().get_data());
-		raise(SIGKILL);
+		// The execvp() function only returns if an error occurs.
+		CRASH_NOW_MSG("Could not create child process: " + p_path);
 	}
 
-	if (p_blocking) {
-		int status;
-		waitpid(pid, &status, 0);
-		if (r_exitcode) {
-			*r_exitcode = WIFEXITED(status) ? WEXITSTATUS(status) : status;
-		}
+	int status;
+	waitpid(pid, &status, 0);
+	if (r_exitcode) {
+		*r_exitcode = WIFEXITED(status) ? WEXITSTATUS(status) : status;
+	}
+	return OK;
+#endif
+}
 
-	} else {
-		if (r_child_id) {
-			*r_child_id = pid;
+Error OS_Unix::create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id) {
+#ifdef __EMSCRIPTEN__
+	// Don't compile this code at all to avoid undefined references.
+	// Actual virtual call goes to OS_JavaScript.
+	ERR_FAIL_V(ERR_BUG);
+#else
+	pid_t pid = fork();
+	ERR_FAIL_COND_V(pid < 0, ERR_CANT_FORK);
+
+	if (pid == 0) {
+		// The new process
+		// Create a new session-ID so parent won't wait for it.
+		// This ensures the process won't go zombie at the end.
+		setsid();
+
+		Vector<char *> args;
+		args.push_back((char *)p_path.utf8().get_data());
+		for (int i = 0; i < p_arguments.size(); i++) {
+			args.push_back((char *)p_arguments[i].utf8().get_data());
 		}
+		args.push_back(0);
+
+		execvp(p_path.utf8().get_data(), &args[0]);
+		// The execvp() function only returns if an error occurs.
+		CRASH_NOW_MSG("Could not create child process: " + p_path);
 	}
 
+	if (r_child_id) {
+		*r_child_id = pid;
+	}
 	return OK;
 #endif
 }

+ 2 - 1
drivers/unix/os_unix.h

@@ -82,7 +82,8 @@ public:
 	virtual void delay_usec(uint32_t p_usec) const override;
 	virtual uint64_t get_ticks_usec() const override;
 
-	virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr) override;
 	virtual Error kill(const ProcessID &p_pid) override;
 	virtual int get_process_id() const override;
 

+ 3 - 6
editor/editor_node.cpp

@@ -2855,8 +2855,7 @@ void EditorNode::_discard_changes(const String &p_str) {
 			args.push_back(exec.get_base_dir());
 			args.push_back("--project-manager");
 
-			OS::ProcessID pid = 0;
-			Error err = OS::get_singleton()->execute(exec, args, false, &pid);
+			Error err = OS::get_singleton()->create_process(exec, args);
 			ERR_FAIL_COND(err);
 		} break;
 	}
@@ -5139,9 +5138,7 @@ void EditorNode::_global_menu_new_window(const Variant &p_tag) {
 		List<String> args;
 		args.push_back("-p");
 		String exec = OS::get_singleton()->get_executable_path();
-
-		OS::ProcessID pid = 0;
-		OS::get_singleton()->execute(exec, args, false, &pid);
+		OS::get_singleton()->create_process(exec, args);
 	}
 }
 
@@ -5467,7 +5464,7 @@ void EditorNode::_print_handler(void *p_this, const String &p_string, bool p_err
 
 static void _execute_thread(void *p_ud) {
 	EditorNode::ExecuteThreadArgs *eta = (EditorNode::ExecuteThreadArgs *)p_ud;
-	Error err = OS::get_singleton()->execute(eta->path, eta->args, true, nullptr, &eta->output, &eta->exitcode, true, &eta->execute_output_mutex);
+	Error err = OS::get_singleton()->execute(eta->path, eta->args, &eta->output, &eta->exitcode, true, &eta->execute_output_mutex);
 	print_verbose("Thread exit status: " + itos(eta->exitcode));
 	if (err != OK) {
 		eta->exitcode = err;

+ 1 - 1
editor/editor_run.cpp

@@ -201,7 +201,7 @@ Error EditorRun::run(const String &p_scene, const String &p_custom_args, const L
 	int instances = EditorSettings::get_singleton()->get_project_metadata("debug_options", "run_debug_instances", 1);
 	for (int i = 0; i < instances; i++) {
 		OS::ProcessID pid = 0;
-		Error err = OS::get_singleton()->execute(exec, args, false, &pid);
+		Error err = OS::get_singleton()->create_process(exec, args, &pid);
 		ERR_FAIL_COND_V(err, err);
 		pids.push_back(pid);
 	}

+ 1 - 1
editor/plugins/script_editor_plugin.cpp

@@ -2211,7 +2211,7 @@ bool ScriptEditor::edit(const RES &p_resource, int p_line, int p_col, bool p_gra
 			args.push_back(script_path);
 		}
 
-		Error err = OS::get_singleton()->execute(path, args, false);
+		Error err = OS::get_singleton()->create_process(path, args);
 		if (err == OK) {
 			return false;
 		}

+ 5 - 14
editor/project_manager.cpp

@@ -1297,9 +1297,7 @@ void ProjectList::_global_menu_new_window(const Variant &p_tag) {
 	List<String> args;
 	args.push_back("-p");
 	String exec = OS::get_singleton()->get_executable_path();
-
-	OS::ProcessID pid = 0;
-	OS::get_singleton()->execute(exec, args, false, &pid);
+	OS::get_singleton()->create_process(exec, args);
 }
 
 void ProjectList::_global_menu_open_project(const Variant &p_tag) {
@@ -1310,9 +1308,7 @@ void ProjectList::_global_menu_open_project(const Variant &p_tag) {
 		List<String> args;
 		args.push_back(conf);
 		String exec = OS::get_singleton()->get_executable_path();
-
-		OS::ProcessID pid = 0;
-		OS::get_singleton()->execute(exec, args, false, &pid);
+		OS::get_singleton()->create_process(exec, args);
 	}
 }
 
@@ -2055,9 +2051,7 @@ void ProjectManager::_open_selected_projects() {
 		}
 
 		String exec = OS::get_singleton()->get_executable_path();
-
-		OS::ProcessID pid = 0;
-		Error err = OS::get_singleton()->execute(exec, args, false, &pid);
+		Error err = OS::get_singleton()->create_process(exec, args);
 		ERR_FAIL_COND(err);
 	}
 
@@ -2143,9 +2137,7 @@ void ProjectManager::_run_project_confirm() {
 		}
 
 		String exec = OS::get_singleton()->get_executable_path();
-
-		OS::ProcessID pid = 0;
-		Error err = OS::get_singleton()->execute(exec, args, false, &pid);
+		Error err = OS::get_singleton()->create_process(exec, args);
 		ERR_FAIL_COND(err);
 	}
 }
@@ -2270,8 +2262,7 @@ void ProjectManager::_language_selected(int p_id) {
 void ProjectManager::_restart_confirm() {
 	List<String> args = OS::get_singleton()->get_cmdline_args();
 	String exec = OS::get_singleton()->get_executable_path();
-	OS::ProcessID pid = 0;
-	Error err = OS::get_singleton()->execute(exec, args, false, &pid);
+	Error err = OS::get_singleton()->create_process(exec, args);
 	ERR_FAIL_COND(err);
 
 	_dim_window();

+ 1 - 2
main/main.cpp

@@ -2686,8 +2686,7 @@ void Main::cleanup() {
 		//attempt to restart with arguments
 		String exec = OS::get_singleton()->get_executable_path();
 		List<String> args = OS::get_singleton()->get_restart_on_exit_arguments();
-		OS::ProcessID pid = 0;
-		OS::get_singleton()->execute(exec, args, false, &pid);
+		OS::get_singleton()->create_process(exec, args);
 		OS::get_singleton()->set_restart_on_exit(false, List<String>()); //clear list (uses memory)
 	}
 

+ 11 - 11
platform/android/export/export.cpp

@@ -308,7 +308,7 @@ class EditorExportPlatformAndroid : public EditorExportPlatform {
 				List<String> args;
 				args.push_back("devices");
 				int ec;
-				OS::get_singleton()->execute(adb, args, true, nullptr, &devices, &ec);
+				OS::get_singleton()->execute(adb, args, &devices, &ec);
 
 				Vector<String> ds = devices.split("\n");
 				Vector<String> ldevices;
@@ -361,7 +361,7 @@ class EditorExportPlatformAndroid : public EditorExportPlatform {
 							int ec2;
 							String dp;
 
-							OS::get_singleton()->execute(adb, args, true, nullptr, &dp, &ec2);
+							OS::get_singleton()->execute(adb, args, &dp, &ec2);
 
 							Vector<String> props = dp.split("\n");
 							String vendor;
@@ -432,7 +432,7 @@ class EditorExportPlatformAndroid : public EditorExportPlatform {
 
 			List<String> args;
 			args.push_back("kill-server");
-			OS::get_singleton()->execute(adb, args, true);
+			OS::get_singleton()->execute(adb, args);
 		};
 	}
 
@@ -1800,7 +1800,7 @@ public:
 			args.push_back("uninstall");
 			args.push_back(get_package_name(package_name));
 
-			err = OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+			err = OS::get_singleton()->execute(adb, args, nullptr, &rv);
 		}
 
 		print_line("Installing to device (please wait...): " + devices[p_device].name);
@@ -1815,7 +1815,7 @@ public:
 		args.push_back("-r");
 		args.push_back(tmp_export_path);
 
-		err = OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+		err = OS::get_singleton()->execute(adb, args, nullptr, &rv);
 		if (err || rv != 0) {
 			EditorNode::add_io_error("Could not install to device.");
 			CLEANUP_AND_RETURN(ERR_CANT_CREATE);
@@ -1832,7 +1832,7 @@ public:
 				args.push_back(devices[p_device].id);
 				args.push_back("reverse");
 				args.push_back("--remove-all");
-				OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+				OS::get_singleton()->execute(adb, args, nullptr, &rv);
 
 				if (p_debug_flags & DEBUG_FLAG_REMOTE_DEBUG) {
 					int dbg_port = EditorSettings::get_singleton()->get("network/debug/remote_port");
@@ -1843,7 +1843,7 @@ public:
 					args.push_back("tcp:" + itos(dbg_port));
 					args.push_back("tcp:" + itos(dbg_port));
 
-					OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+					OS::get_singleton()->execute(adb, args, nullptr, &rv);
 					print_line("Reverse result: " + itos(rv));
 				}
 
@@ -1857,7 +1857,7 @@ public:
 					args.push_back("tcp:" + itos(fs_port));
 					args.push_back("tcp:" + itos(fs_port));
 
-					err = OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+					err = OS::get_singleton()->execute(adb, args, nullptr, &rv);
 					print_line("Reverse result2: " + itos(rv));
 				}
 			} else {
@@ -1885,7 +1885,7 @@ public:
 		args.push_back("-n");
 		args.push_back(get_package_name(package_name) + "/com.godot.game.GodotApp");
 
-		err = OS::get_singleton()->execute(adb, args, true, nullptr, nullptr, &rv);
+		err = OS::get_singleton()->execute(adb, args, nullptr, &rv);
 		if (err || rv != 0) {
 			EditorNode::add_io_error("Could not execute on device.");
 			CLEANUP_AND_RETURN(ERR_CANT_CREATE);
@@ -2288,7 +2288,7 @@ public:
 		args.push_back(user);
 		args.push_back(export_path);
 		int retval;
-		OS::get_singleton()->execute(apksigner, args, true, NULL, NULL, &retval);
+		OS::get_singleton()->execute(apksigner, args, nullptr, &retval);
 		if (retval) {
 			EditorNode::add_io_error("'apksigner' returned with error #" + itos(retval));
 			return ERR_CANT_CREATE;
@@ -2303,7 +2303,7 @@ public:
 		args.push_back("--verbose");
 		args.push_back(export_path);
 
-		OS::get_singleton()->execute(apksigner, args, true, NULL, NULL, &retval);
+		OS::get_singleton()->execute(apksigner, args, nullptr, &retval);
 		if (retval) {
 			EditorNode::add_io_error("'apksigner' verification of " + export_label + " failed.");
 			return ERR_CANT_CREATE;

+ 4 - 4
platform/iphone/export/export.cpp

@@ -979,7 +979,7 @@ Error EditorExportPlatformIOS::_codesign(String p_file, void *p_userdata) {
 		codesign_args.push_back("-s");
 		codesign_args.push_back(data->preset->get(data->debug ? "application/code_sign_identity_debug" : "application/code_sign_identity_release"));
 		codesign_args.push_back(p_file);
-		return OS::get_singleton()->execute("codesign", codesign_args, true);
+		return OS::get_singleton()->execute("codesign", codesign_args);
 	}
 	return OK;
 }
@@ -1229,7 +1229,7 @@ Error EditorExportPlatformIOS::_copy_asset(const String &p_out_dir, const String
 			install_name_args.push_back(String("@rpath").plus_file(framework_name).plus_file(file_name));
 			install_name_args.push_back(destination);
 
-			OS::get_singleton()->execute("install_name_tool", install_name_args, true);
+			OS::get_singleton()->execute("install_name_tool", install_name_args);
 		}
 
 		// Creating Info.plist
@@ -1848,7 +1848,7 @@ Error EditorExportPlatformIOS::export_project(const Ref<EditorExportPreset> &p_p
 	archive_args.push_back("archive");
 	archive_args.push_back("-archivePath");
 	archive_args.push_back(archive_path);
-	err = OS::get_singleton()->execute("xcodebuild", archive_args, true);
+	err = OS::get_singleton()->execute("xcodebuild", archive_args);
 	ERR_FAIL_COND_V(err, err);
 
 	if (ep.step("Making .ipa", 4)) {
@@ -1863,7 +1863,7 @@ Error EditorExportPlatformIOS::export_project(const Ref<EditorExportPreset> &p_p
 	export_args.push_back("-allowProvisioningUpdates");
 	export_args.push_back("-exportPath");
 	export_args.push_back(dest_dir);
-	err = OS::get_singleton()->execute("xcodebuild", export_args, true);
+	err = OS::get_singleton()->execute("xcodebuild", export_args);
 	ERR_FAIL_COND_V(err, err);
 #else
 	print_line(".ipa can only be built on macOS. Leaving Xcode project without building the package.");

+ 6 - 2
platform/javascript/os_javascript.cpp

@@ -106,14 +106,18 @@ void OS_JavaScript::finalize() {
 
 // Miscellaneous
 
-Error OS_JavaScript::execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+Error OS_JavaScript::execute(const String &p_path, const List<String> &p_arguments, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+	return create_process(p_path, p_arguments);
+}
+
+Error OS_JavaScript::create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id) {
 	Array args;
 	for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
 		args.push_back(E->get());
 	}
 	String json_args = JSON::print(args);
 	int failed = godot_js_os_execute(json_args.utf8().get_data());
-	ERR_FAIL_COND_V_MSG(failed, ERR_UNAVAILABLE, "OS::execute() must be implemented in JavaScript via 'engine.setOnExecute' if required.");
+	ERR_FAIL_COND_V_MSG(failed, ERR_UNAVAILABLE, "OS::execute() or create_process() must be implemented in JavaScript via 'engine.setOnExecute' if required.");
 	return OK;
 }
 

+ 2 - 1
platform/javascript/os_javascript.h

@@ -70,7 +70,8 @@ public:
 	MainLoop *get_main_loop() const override;
 	bool main_loop_iterate();
 
-	Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr) override;
 	Error kill(const ProcessID &p_pid) override;
 	int get_process_id() const override;
 

+ 1 - 1
platform/linuxbsd/crash_handler_linuxbsd.cpp

@@ -104,7 +104,7 @@ static void handle_crash(int sig) {
 
 			// Try to get the file/line number using addr2line
 			int ret;
-			Error err = OS::get_singleton()->execute(String("addr2line"), args, true, nullptr, &output, &ret);
+			Error err = OS::get_singleton()->execute(String("addr2line"), args, &output, &ret);
 			if (err == OK) {
 				output.erase(output.length() - 1, 1);
 			}

+ 1 - 1
platform/linuxbsd/display_server_x11.cpp

@@ -191,7 +191,7 @@ void DisplayServerX11::alert(const String &p_alert, const String &p_title) {
 	}
 
 	if (program.length()) {
-		OS::get_singleton()->execute(program, args, true);
+		OS::get_singleton()->execute(program, args);
 	} else {
 		print_line(p_alert);
 	}

+ 10 - 10
platform/linuxbsd/os_linuxbsd.cpp

@@ -128,7 +128,7 @@ Error OS_LinuxBSD::shell_open(String p_uri) {
 	args.push_back(p_uri);
 
 	// Agnostic
-	ok = execute("xdg-open", args, true, nullptr, nullptr, &err_code);
+	ok = execute("xdg-open", args, nullptr, &err_code);
 	if (ok == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
@@ -136,25 +136,25 @@ Error OS_LinuxBSD::shell_open(String p_uri) {
 	}
 	// GNOME
 	args.push_front("open"); // The command is `gio open`, so we need to add it to args
-	ok = execute("gio", args, true, nullptr, nullptr, &err_code);
+	ok = execute("gio", args, nullptr, &err_code);
 	if (ok == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
 		return ERR_FILE_NOT_FOUND;
 	}
 	args.pop_front();
-	ok = execute("gvfs-open", args, true, nullptr, nullptr, &err_code);
+	ok = execute("gvfs-open", args, nullptr, &err_code);
 	if (ok == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
 		return ERR_FILE_NOT_FOUND;
 	}
 	// KDE
-	ok = execute("kde-open5", args, true, nullptr, nullptr, &err_code);
+	ok = execute("kde-open5", args, nullptr, &err_code);
 	if (ok == OK && !err_code) {
 		return OK;
 	}
-	ok = execute("kde-open", args, true, nullptr, nullptr, &err_code);
+	ok = execute("kde-open", args, nullptr, &err_code);
 	return !err_code ? ok : FAILED;
 }
 
@@ -232,7 +232,7 @@ String OS_LinuxBSD::get_system_dir(SystemDir p_dir) const {
 	String pipe;
 	List<String> arg;
 	arg.push_back(xdgparam);
-	Error err = const_cast<OS_LinuxBSD *>(this)->execute("xdg-user-dir", arg, true, nullptr, &pipe);
+	Error err = const_cast<OS_LinuxBSD *>(this)->execute("xdg-user-dir", arg, &pipe);
 	if (err != OK) {
 		return ".";
 	}
@@ -307,7 +307,7 @@ Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 	List<String> args;
 	args.push_back(p_path);
 	args.push_front("trash"); // The command is `gio trash <file_name>` so we need to add it to args.
-	Error result = execute("gio", args, true, nullptr, nullptr, &err_code); // For GNOME based machines.
+	Error result = execute("gio", args, nullptr, &err_code); // For GNOME based machines.
 	if (result == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
@@ -317,7 +317,7 @@ Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 	args.pop_front();
 	args.push_front("move");
 	args.push_back("trash:/"); // The command is `kioclient5 move <file_name> trash:/`.
-	result = execute("kioclient5", args, true, nullptr, nullptr, &err_code); // For KDE based machines.
+	result = execute("kioclient5", args, nullptr, &err_code); // For KDE based machines.
 	if (result == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
@@ -326,7 +326,7 @@ Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 
 	args.pop_front();
 	args.pop_back();
-	result = execute("gvfs-trash", args, true, nullptr, nullptr, &err_code); // For older Linux machines.
+	result = execute("gvfs-trash", args, nullptr, &err_code); // For older Linux machines.
 	if (result == OK && !err_code) {
 		return OK;
 	} else if (err_code == 2) {
@@ -432,7 +432,7 @@ Error OS_LinuxBSD::move_to_trash(const String &p_path) {
 	mv_args.push_back(trash_path + "/files");
 	{
 		int retval;
-		Error err = execute("mv", mv_args, true, nullptr, nullptr, &retval);
+		Error err = execute("mv", mv_args, nullptr, &retval);
 
 		// Issue an error if "mv" failed to move the given resource to the trash can.
 		if (err != OK || retval != 0) {

+ 1 - 1
platform/osx/crash_handler_osx.mm

@@ -135,7 +135,7 @@ static void handle_crash(int sig) {
 
 				int ret;
 				String out = "";
-				Error err = OS::get_singleton()->execute(String("atos"), args, true, NULL, &out, &ret);
+				Error err = OS::get_singleton()->execute(String("atos"), args, &out, &ret);
 				if (err == OK && out.substr(0, 2) != "0x") {
 					out.erase(out.length() - 1, 1);
 					output = out;

+ 1 - 3
platform/osx/display_server_osx.mm

@@ -256,9 +256,7 @@ static NSCursor *_cursorFromSelector(SEL selector, SEL fallback = nil) {
 		List<String> args;
 		args.push_back(((OS_OSX *)(OS_OSX::get_singleton()))->open_with_filename);
 		String exec = OS::get_singleton()->get_executable_path();
-
-		OS::ProcessID pid = 0;
-		OS::get_singleton()->execute(exec, args, false, &pid);
+		OS::get_singleton()->create_process(exec, args);
 	}
 #endif
 	return YES;

+ 3 - 3
platform/osx/export/export.cpp

@@ -419,7 +419,7 @@ Error EditorExportPlatformOSX::_notarize(const Ref<EditorExportPreset> &p_preset
 	args.push_back(p_path);
 
 	String str;
-	Error err = OS::get_singleton()->execute("xcrun", args, true, nullptr, &str, nullptr, true);
+	Error err = OS::get_singleton()->execute("xcrun", args, &str, nullptr, true);
 	ERR_FAIL_COND_V(err != OK, err);
 
 	print_line("altool (" + p_path + "):\n" + str);
@@ -470,7 +470,7 @@ Error EditorExportPlatformOSX::_code_sign(const Ref<EditorExportPreset> &p_prese
 	args.push_back(p_path);
 
 	String str;
-	Error err = OS::get_singleton()->execute("codesign", args, true, nullptr, &str, nullptr, true);
+	Error err = OS::get_singleton()->execute("codesign", args, &str, nullptr, true);
 	ERR_FAIL_COND_V(err != OK, err);
 
 	print_line("codesign (" + p_path + "):\n" + str);
@@ -504,7 +504,7 @@ Error EditorExportPlatformOSX::_create_dmg(const String &p_dmg_path, const Strin
 	args.push_back(p_app_path_name);
 
 	String str;
-	Error err = OS::get_singleton()->execute("hdiutil", args, true, nullptr, &str, nullptr, true);
+	Error err = OS::get_singleton()->execute("hdiutil", args, &str, nullptr, true);
 	ERR_FAIL_COND_V(err != OK, err);
 
 	print_line("hdiutil returned: " + str);

+ 1 - 1
platform/uwp/export/export.cpp

@@ -1411,7 +1411,7 @@ public:
 		args.push_back(cert_pass);
 		args.push_back(p_path);
 
-		OS::get_singleton()->execute(signtool_path, args, true);
+		OS::get_singleton()->execute(signtool_path, args);
 #endif // WINDOWS_ENABLED
 
 		return OK;

+ 5 - 1
platform/uwp/os_uwp.cpp

@@ -638,7 +638,11 @@ void OS_UWP::set_custom_mouse_cursor(const RES &p_cursor, CursorShape p_shape, c
 	// TODO
 }
 
-Error OS_UWP::execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+Error OS_UWP::execute(const String &p_path, const List<String> &p_arguments, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+	return FAILED;
+};
+
+Error OS_UWP::create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id) {
 	return FAILED;
 };
 

+ 2 - 1
platform/uwp/os_uwp.h

@@ -199,7 +199,8 @@ public:
 	virtual void delay_usec(uint32_t p_usec) const;
 	virtual uint64_t get_ticks_usec() const;
 
-	virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr);
+	virtual Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr);
+	virtual Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr);
 	virtual Error kill(const ProcessID &p_pid);
 
 	virtual bool has_environment(const String &p_var) const;

+ 3 - 3
platform/windows/export/export.cpp

@@ -173,11 +173,11 @@ void EditorExportPlatformWindows::_rcedit_add_data(const Ref<EditorExportPreset>
 	}
 
 #ifdef WINDOWS_ENABLED
-	OS::get_singleton()->execute(rcedit_path, args, true);
+	OS::get_singleton()->execute(rcedit_path, args);
 #else
 	// On non-Windows we need WINE to run rcedit
 	args.push_front(rcedit_path);
-	OS::get_singleton()->execute(wine_path, args, true);
+	OS::get_singleton()->execute(wine_path, args);
 #endif
 }
 
@@ -314,7 +314,7 @@ Error EditorExportPlatformWindows::_code_sign(const Ref<EditorExportPreset> &p_p
 #endif
 
 	String str;
-	Error err = OS::get_singleton()->execute(signtool_path, args, true, nullptr, &str, nullptr, true);
+	Error err = OS::get_singleton()->execute(signtool_path, args, &str, nullptr, true);
 	ERR_FAIL_COND_V(err != OK, err);
 
 	print_line("codesign (" + p_path + "): " + str);

+ 46 - 39
platform/windows/os_windows.cpp

@@ -410,24 +410,23 @@ String OS_Windows::_quote_command_line_argument(const String &p_text) const {
 	return p_text;
 }
 
-Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments, bool p_blocking, ProcessID *r_child_id, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
+Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments, String *r_pipe, int *r_exitcode, bool read_stderr, Mutex *p_pipe_mutex) {
 	String path = p_path.replace("/", "\\");
+	String command = _quote_command_line_argument(path);
+	for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
+		command += " " + _quote_command_line_argument(E->get());
+	}
 
-	if (p_blocking && r_pipe) {
-		String argss = _quote_command_line_argument(path);
-		for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
-			argss += " " + _quote_command_line_argument(E->get());
-		}
-
+	if (r_pipe) {
 		if (read_stderr) {
-			argss += " 2>&1"; // Read stderr too
+			command += " 2>&1"; // Include stderr
 		}
-		// Note: _wpopen is calling command as "cmd.exe /c argss", instead of executing it directly, add extra quotes around full command, to prevent it from stripping quotes in the command.
-		argss = _quote_command_line_argument(argss);
-
-		FILE *f = _wpopen((LPCWSTR)(argss.utf16().get_data()), L"r");
-		ERR_FAIL_COND_V(!f, ERR_CANT_OPEN);
+		// Add extra quotes around the full command, to prevent it from stripping quotes in the command,
+		// because _wpopen calls command as "cmd.exe /c command", instead of executing it directly
+		command = _quote_command_line_argument(command);
 
+		FILE *f = _wpopen((LPCWSTR)(command.utf16().get_data()), L"r");
+		ERR_FAIL_COND_V_MSG(!f, ERR_CANT_OPEN, "Cannot create pipe from command: " + command);
 		char buf[65535];
 		while (fgets(buf, 65535, f)) {
 			if (p_pipe_mutex) {
@@ -438,20 +437,40 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 				p_pipe_mutex->unlock();
 			}
 		}
-
 		int rv = _pclose(f);
+
 		if (r_exitcode) {
 			*r_exitcode = rv;
 		}
-
 		return OK;
 	}
 
-	String cmdline = _quote_command_line_argument(path);
-	const List<String>::Element *I = p_arguments.front();
-	while (I) {
-		cmdline += " " + _quote_command_line_argument(I->get());
-		I = I->next();
+	ProcessInfo pi;
+	ZeroMemory(&pi.si, sizeof(pi.si));
+	pi.si.cb = sizeof(pi.si);
+	ZeroMemory(&pi.pi, sizeof(pi.pi));
+	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
+
+	int ret = CreateProcessW(nullptr, (LPWSTR)(command.utf16().ptrw()), nullptr, nullptr, false, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
+	ERR_FAIL_COND_V_MSG(ret == 0, ERR_CANT_FORK, "Could not create child process: " + command);
+
+	WaitForSingleObject(pi.pi.hProcess, INFINITE);
+	if (r_exitcode) {
+		DWORD ret2;
+		GetExitCodeProcess(pi.pi.hProcess, &ret2);
+		*r_exitcode = ret2;
+	}
+	CloseHandle(pi.pi.hProcess);
+	CloseHandle(pi.pi.hThread);
+
+	return OK;
+};
+
+Error OS_Windows::create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id) {
+	String path = p_path.replace("/", "\\");
+	String command = _quote_command_line_argument(path);
+	for (const List<String>::Element *E = p_arguments.front(); E; E = E->next()) {
+		command += " " + _quote_command_line_argument(E->get());
 	}
 
 	ProcessInfo pi;
@@ -460,27 +479,15 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 	ZeroMemory(&pi.pi, sizeof(pi.pi));
 	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
 
-	Char16String modstr = cmdline.utf16(); // Windows wants to change this no idea why.
-	int ret = CreateProcessW(nullptr, (LPWSTR)(modstr.ptrw()), nullptr, nullptr, 0, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
-	ERR_FAIL_COND_V(ret == 0, ERR_CANT_FORK);
+	int ret = CreateProcessW(nullptr, (LPWSTR)(command.utf16().ptrw()), nullptr, nullptr, false, NORMAL_PRIORITY_CLASS & CREATE_NO_WINDOW, nullptr, nullptr, si_w, &pi.pi);
+	ERR_FAIL_COND_V_MSG(ret == 0, ERR_CANT_FORK, "Could not create child process: " + command);
 
-	if (p_blocking) {
-		WaitForSingleObject(pi.pi.hProcess, INFINITE);
-		if (r_exitcode) {
-			DWORD ret2;
-			GetExitCodeProcess(pi.pi.hProcess, &ret2);
-			*r_exitcode = ret2;
-		}
-
-		CloseHandle(pi.pi.hProcess);
-		CloseHandle(pi.pi.hThread);
-	} else {
-		ProcessID pid = pi.pi.dwProcessId;
-		if (r_child_id) {
-			*r_child_id = pid;
-		}
-		process_map->insert(pid, pi);
+	ProcessID pid = pi.pi.dwProcessId;
+	if (r_child_id) {
+		*r_child_id = pid;
 	}
+	process_map->insert(pid, pi);
+
 	return OK;
 };
 

+ 2 - 1
platform/windows/os_windows.h

@@ -136,7 +136,8 @@ public:
 	virtual void delay_usec(uint32_t p_usec) const override;
 	virtual uint64_t get_ticks_usec() const override;
 
-	virtual Error execute(const String &p_path, const List<String> &p_arguments, bool p_blocking = true, ProcessID *r_child_id = nullptr, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error execute(const String &p_path, const List<String> &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr) override;
+	virtual Error create_process(const String &p_path, const List<String> &p_arguments, ProcessID *r_child_id = nullptr) override;
 	virtual Error kill(const ProcessID &p_pid) override;
 	virtual int get_process_id() const override;
 

+ 1 - 1
tests/test_math.cpp

@@ -617,7 +617,7 @@ MainLoop *test() {
 
 	List<String> args;
 	args.push_back("-l");
-	Error err = OS::get_singleton()->execute("/bin/ls", args, true, nullptr, &ret);
+	Error err = OS::get_singleton()->execute("/bin/ls", args, &ret);
 	print_line("error: " + itos(err));
 	print_line(ret);