From 80e66a6214919e755f61076b8f12ed4142476201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Wed, 30 May 2018 12:18:01 +0200 Subject: [PATCH] Improve return value of OS.execute in blocking/non-blocking variants Initialized the PID to -2, which will be the value returns in blocking- mode where the PID is not available. (-1 was already taken to signify an execution failure). OS::execute will now properly return a non-OK error code when it fails to execute the target file. The documentation was rewritten to be very clear about the differences between blocking and non-blocking mode. Fixes #19056. (cherry picked from commit f392650be2434aadad71af95a0b81a33900e84ec) --- core/bind/core_bind.cpp | 3 ++- doc/classes/OS.xml | 24 ++++++++++++++++-------- platform/windows/os_windows.cpp | 7 ------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/core/bind/core_bind.cpp b/core/bind/core_bind.cpp index 60b4ef1753d..da7c52757b3 100644 --- a/core/bind/core_bind.cpp +++ b/core/bind/core_bind.cpp @@ -375,7 +375,7 @@ Error _OS::shell_open(String p_uri) { int _OS::execute(const String &p_path, const Vector &p_arguments, bool p_blocking, Array p_output) { - OS::ProcessID pid; + OS::ProcessID pid = -2; List args; for (int i = 0; i < p_arguments.size(); i++) args.push_back(p_arguments[i]); @@ -388,6 +388,7 @@ int _OS::execute(const String &p_path, const Vector &p_arguments, bool p else return pid; } + Error _OS::kill(int p_pid) { return OS::get_singleton()->kill(p_pid); diff --git a/doc/classes/OS.xml b/doc/classes/OS.xml index 0b6b64dbe96..f08cd7063ba 100644 --- a/doc/classes/OS.xml +++ b/doc/classes/OS.xml @@ -94,17 +94,24 @@ - Execute the file at the given path, optionally blocking until it returns. - Platform path resolution will take place. The resolved file must exist and be executable. - Returns a process id. - For example: + 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', ['-c', '3', 'godotengine.org'])[/code] will resolve to [code]ping -c 3 godotengine.org[/code] in the system's shell. + This method has slightly different behaviour based on whether the [code]blocking[/code] mode is enabled. + When [code]blocking[/code] is enabled, 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. + When [code]blocking[/code] is disabled, 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 -2 (no process ID information is available in blocking mode). 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 -1. + Example of blocking mode and retrieving the shell output: [codeblock] var output = [] - var pid = OS.execute('ls', [], true, output) + OS.execute('ls', ['-l', '/tmp'], true, output) [/codeblock] - If you wish to access a shell built-in or perform a composite command, a platform specific shell can be invoked. For example: + Example of non-blocking mode, running another instance of the project and storing its process ID: [codeblock] - var pid = OS.execute('CMD.exe', ['/C', 'cd %TEMP% && dir'], true, output) + var pid = OS.execute(OS.get_executable_path(), [], false) + [/codeblock] + If you wish to access a shell built-in or perform a composite command, a platform-specific shell can be invoked. For example: + [codeblock] + OS.execute('CMD.exe', ['/C', 'cd %TEMP% && dir'], true, output) [/codeblock] @@ -495,7 +502,8 @@ - Kill a process ID (this method can be used to kill processes that were not spawned by the game). + Kill (terminate) the process identified by the given process ID ([code]pid[/code]), e.g. the one returned by [method execute] in non-blocking mode. + Note that this method can also be used to kill processes that were not spawned by the game. diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp index 4a2d0727493..ae8f718fb01 100644 --- a/platform/windows/os_windows.cpp +++ b/platform/windows/os_windows.cpp @@ -2020,10 +2020,6 @@ Error OS_Windows::execute(const String &p_path, const List &p_arguments, argss += String(" \"") + E->get() + "\""; } - //print_line("ARGS: "+argss); - //argss+"\""; - //argss+=" 2>nul"; - FILE *f = _wpopen(argss.c_str(), L"r"); ERR_FAIL_COND_V(!f, ERR_CANT_OPEN); @@ -2050,15 +2046,12 @@ Error OS_Windows::execute(const String &p_path, const List &p_arguments, I = I->next(); }; - //cmdline+="\""; - 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; - print_line("running cmdline: " + cmdline); Vector modstr; //windows wants to change this no idea why modstr.resize(cmdline.size()); for (int i = 0; i < cmdline.size(); i++)