Browse Source

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.
Rémi Verschelde 7 years ago
parent
commit
f392650be2
3 changed files with 18 additions and 16 deletions
  1. 2 1
      core/bind/core_bind.cpp
  2. 16 8
      doc/classes/OS.xml
  3. 0 7
      platform/windows/os_windows.cpp

+ 2 - 1
core/bind/core_bind.cpp

@@ -399,7 +399,7 @@ Error _OS::shell_open(String p_uri) {
 
 
 int _OS::execute(const String &p_path, const Vector<String> &p_arguments, bool p_blocking, Array p_output) {
 int _OS::execute(const String &p_path, const Vector<String> &p_arguments, bool p_blocking, Array p_output) {
 
 
-	OS::ProcessID pid;
+	OS::ProcessID pid = -2;
 	List<String> args;
 	List<String> args;
 	for (int i = 0; i < p_arguments.size(); i++)
 	for (int i = 0; i < p_arguments.size(); i++)
 		args.push_back(p_arguments[i]);
 		args.push_back(p_arguments[i]);
@@ -412,6 +412,7 @@ int _OS::execute(const String &p_path, const Vector<String> &p_arguments, bool p
 	else
 	else
 		return pid;
 		return pid;
 }
 }
+
 Error _OS::kill(int p_pid) {
 Error _OS::kill(int p_pid) {
 
 
 	return OS::get_singleton()->kill(p_pid);
 	return OS::get_singleton()->kill(p_pid);

+ 16 - 8
doc/classes/OS.xml

@@ -94,17 +94,24 @@
 			<argument index="3" name="output" type="Array" default="[  ]">
 			<argument index="3" name="output" type="Array" default="[  ]">
 			</argument>
 			</argument>
 			<description>
 			<description>
-				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]
 				[codeblock]
 				var output = []
 				var output = []
-				var pid = OS.execute('ls', [], true, output)
+				OS.execute('ls', ['-l', '/tmp'], true, output)
 				[/codeblock]
 				[/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]
 				[codeblock]
-				var pid = OS.execute('CMD.exe', ['/C', 'cd %TEMP% &amp;&amp; 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% &amp;&amp; dir'], true, output)
 				[/codeblock]
 				[/codeblock]
 			</description>
 			</description>
 		</method>
 		</method>
@@ -527,7 +534,8 @@
 			<argument index="0" name="pid" type="int">
 			<argument index="0" name="pid" type="int">
 			</argument>
 			</argument>
 			<description>
 			<description>
-				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.
 			</description>
 			</description>
 		</method>
 		</method>
 		<method name="native_video_is_playing">
 		<method name="native_video_is_playing">

+ 0 - 7
platform/windows/os_windows.cpp

@@ -2209,10 +2209,6 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 			argss += String(" \"") + E->get() + "\"";
 			argss += String(" \"") + E->get() + "\"";
 		}
 		}
 
 
-		//print_line("ARGS: "+argss);
-		//argss+"\"";
-		//argss+=" 2>nul";
-
 		FILE *f = _wpopen(argss.c_str(), L"r");
 		FILE *f = _wpopen(argss.c_str(), L"r");
 
 
 		ERR_FAIL_COND_V(!f, ERR_CANT_OPEN);
 		ERR_FAIL_COND_V(!f, ERR_CANT_OPEN);
@@ -2239,15 +2235,12 @@ Error OS_Windows::execute(const String &p_path, const List<String> &p_arguments,
 		I = I->next();
 		I = I->next();
 	};
 	};
 
 
-	//cmdline+="\"";
-
 	ProcessInfo pi;
 	ProcessInfo pi;
 	ZeroMemory(&pi.si, sizeof(pi.si));
 	ZeroMemory(&pi.si, sizeof(pi.si));
 	pi.si.cb = sizeof(pi.si);
 	pi.si.cb = sizeof(pi.si);
 	ZeroMemory(&pi.pi, sizeof(pi.pi));
 	ZeroMemory(&pi.pi, sizeof(pi.pi));
 	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
 	LPSTARTUPINFOW si_w = (LPSTARTUPINFOW)&pi.si;
 
 
-	print_line("running cmdline: " + cmdline);
 	Vector<CharType> modstr; //windows wants to change this no idea why
 	Vector<CharType> modstr; //windows wants to change this no idea why
 	modstr.resize(cmdline.size());
 	modstr.resize(cmdline.size());
 	for (int i = 0; i < cmdline.size(); i++)
 	for (int i = 0; i < cmdline.size(); i++)