From 10f3c1f5870ba1351d565218930d7dd455cce7a9 Mon Sep 17 00:00:00 2001 From: bruvzg <7645683+bruvzg@users.noreply.github.com> Date: Tue, 16 Jul 2024 14:38:01 +0300 Subject: [PATCH] Add support for non-blocking IO mode to `OS.execute_with_pipe`. --- core/core_bind.compat.inc | 16 ++++++++++++++-- core/core_bind.cpp | 6 +++--- core/core_bind.h | 8 +++++++- core/os/os.h | 2 +- doc/classes/OS.xml | 2 ++ drivers/unix/file_access_unix_pipe.cpp | 16 ++++++++++++---- drivers/unix/file_access_unix_pipe.h | 2 +- drivers/unix/os_unix.cpp | 6 +++--- drivers/unix/os_unix.h | 2 +- drivers/windows/file_access_windows_pipe.cpp | 12 +++++++++--- drivers/windows/file_access_windows_pipe.h | 2 +- .../extension_api_validation/4.3-stable.expected | 7 +++++++ platform/web/os_web.cpp | 2 +- platform/web/os_web.h | 2 +- platform/windows/os_windows.cpp | 6 +++--- platform/windows/os_windows.h | 2 +- 16 files changed, 67 insertions(+), 26 deletions(-) diff --git a/core/core_bind.compat.inc b/core/core_bind.compat.inc index 83b7b33e383..3e8ac3c5de6 100644 --- a/core/core_bind.compat.inc +++ b/core/core_bind.compat.inc @@ -32,6 +32,8 @@ namespace core_bind { +// Semaphore + void Semaphore::_post_bind_compat_93605() { post(1); } @@ -40,6 +42,16 @@ void Semaphore::_bind_compatibility_methods() { ClassDB::bind_compatibility_method(D_METHOD("post"), &Semaphore::_post_bind_compat_93605); } -}; // namespace core_bind +// OS -#endif +Dictionary OS::_execute_with_pipe_bind_compat_94434(const String &p_path, const Vector &p_arguments) { + return execute_with_pipe(p_path, p_arguments, true); +} + +void OS::_bind_compatibility_methods() { + ClassDB::bind_compatibility_method(D_METHOD("execute_with_pipe", "path", "arguments"), &OS::_execute_with_pipe_bind_compat_94434); +} + +} // namespace core_bind + +#endif // DISABLE_DEPRECATED diff --git a/core/core_bind.cpp b/core/core_bind.cpp index 4172793f9d8..750598ab201 100644 --- a/core/core_bind.cpp +++ b/core/core_bind.cpp @@ -314,12 +314,12 @@ int OS::execute(const String &p_path, const Vector &p_arguments, Array r return exitcode; } -Dictionary OS::execute_with_pipe(const String &p_path, const Vector &p_arguments) { +Dictionary OS::execute_with_pipe(const String &p_path, const Vector &p_arguments, bool p_blocking) { List args; for (const String &arg : p_arguments) { args.push_back(arg); } - return ::OS::get_singleton()->execute_with_pipe(p_path, args); + return ::OS::get_singleton()->execute_with_pipe(p_path, args, p_blocking); } int OS::create_instance(const Vector &p_arguments) { @@ -619,7 +619,7 @@ void OS::_bind_methods() { ClassDB::bind_method(D_METHOD("get_executable_path"), &OS::get_executable_path); ClassDB::bind_method(D_METHOD("read_string_from_stdin"), &OS::read_string_from_stdin); ClassDB::bind_method(D_METHOD("execute", "path", "arguments", "output", "read_stderr", "open_console"), &OS::execute, DEFVAL(Array()), DEFVAL(false), DEFVAL(false)); - ClassDB::bind_method(D_METHOD("execute_with_pipe", "path", "arguments"), &OS::execute_with_pipe); + ClassDB::bind_method(D_METHOD("execute_with_pipe", "path", "arguments", "blocking"), &OS::execute_with_pipe, DEFVAL(true)); ClassDB::bind_method(D_METHOD("create_process", "path", "arguments", "open_console"), &OS::create_process, DEFVAL(false)); ClassDB::bind_method(D_METHOD("create_instance", "arguments"), &OS::create_instance); ClassDB::bind_method(D_METHOD("kill", "pid"), &OS::kill); diff --git a/core/core_bind.h b/core/core_bind.h index 122963e634d..e4ba0e8f566 100644 --- a/core/core_bind.h +++ b/core/core_bind.h @@ -128,6 +128,12 @@ protected: static void _bind_methods(); static OS *singleton; +#ifndef DISABLE_DEPRECATED + Dictionary _execute_with_pipe_bind_compat_94434(const String &p_path, const Vector &p_arguments); + + static void _bind_compatibility_methods(); +#endif + public: enum RenderingDriver { RENDERING_DRIVER_VULKAN, @@ -161,7 +167,7 @@ public: String get_executable_path() const; String read_string_from_stdin(); int execute(const String &p_path, const Vector &p_arguments, Array r_output = Array(), bool p_read_stderr = false, bool p_open_console = false); - Dictionary execute_with_pipe(const String &p_path, const Vector &p_arguments); + Dictionary execute_with_pipe(const String &p_path, const Vector &p_arguments, bool p_blocking = true); int create_process(const String &p_path, const Vector &p_arguments, bool p_open_console = false); int create_instance(const Vector &p_arguments); Error kill(int p_pid); diff --git a/core/os/os.h b/core/os/os.h index 553ae4cf762..30d2a4266fe 100644 --- a/core/os/os.h +++ b/core/os/os.h @@ -182,7 +182,7 @@ public: virtual Vector get_system_font_path_for_text(const String &p_font_name, const String &p_text, const String &p_locale = String(), const String &p_script = String(), int p_weight = 400, int p_stretch = 100, bool p_italic = false) const { return Vector(); }; virtual String get_executable_path() const; virtual Error execute(const String &p_path, const List &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr, bool p_open_console = false) = 0; - virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments) { return Dictionary(); } + virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking = true) { return Dictionary(); } virtual Error create_process(const String &p_path, const List &p_arguments, ProcessID *r_child_id = nullptr, bool p_open_console = false) = 0; virtual Error create_instance(const List &p_arguments, ProcessID *r_child_id = nullptr) { return create_process(get_executable_path(), p_arguments, r_child_id); }; virtual Error kill(const ProcessID &p_pid) = 0; diff --git a/doc/classes/OS.xml b/doc/classes/OS.xml index 9675f5af507..777950c0752 100644 --- a/doc/classes/OS.xml +++ b/doc/classes/OS.xml @@ -132,8 +132,10 @@ + Creates a new process that runs independently of Godot with redirected IO. It will not terminate when Godot terminates. The path specified in [param path] must exist and be an executable file or macOS [code].app[/code] bundle. The path is resolved based on the current platform. The [param arguments] are used in the given order and separated by a space. + If [param blocking] is [code]false[/code], created pipes work in non-blocking mode, i.e. read and write operations will return immediately. Use [method FileAccess.get_error] to check if the last read/write operation was successful. If the process cannot be created, this method returns an empty [Dictionary]. Otherwise, this method returns a [Dictionary] with the following keys: - [code]"stdio"[/code] - [FileAccess] to access the process stdin and stdout pipes (read/write). - [code]"stderr"[/code] - [FileAccess] to access the process stderr pipe (read only). diff --git a/drivers/unix/file_access_unix_pipe.cpp b/drivers/unix/file_access_unix_pipe.cpp index 34758e8c7d9..0a78429dec2 100644 --- a/drivers/unix/file_access_unix_pipe.cpp +++ b/drivers/unix/file_access_unix_pipe.cpp @@ -41,7 +41,7 @@ #include #include -Error FileAccessUnixPipe::open_existing(int p_rfd, int p_wfd) { +Error FileAccessUnixPipe::open_existing(int p_rfd, int p_wfd, bool p_blocking) { // Open pipe using handles created by pipe(fd) call in the OS.execute_with_pipe. _close(); @@ -51,6 +51,11 @@ Error FileAccessUnixPipe::open_existing(int p_rfd, int p_wfd) { fd[0] = p_rfd; fd[1] = p_wfd; + if (!p_blocking) { + fcntl(fd[0], F_SETFL, fcntl(fd[0], F_GETFL) | O_NONBLOCK); + fcntl(fd[1], F_SETFL, fcntl(fd[1], F_GETFL) | O_NONBLOCK); + } + last_error = OK; return OK; } @@ -74,7 +79,7 @@ Error FileAccessUnixPipe::open_internal(const String &p_path, int p_mode_flags) ERR_FAIL_COND_V_MSG(!S_ISFIFO(st.st_mode), ERR_ALREADY_IN_USE, "Pipe name is already used by file."); } - int f = ::open(path.utf8().get_data(), O_RDWR | O_CLOEXEC); + int f = ::open(path.utf8().get_data(), O_RDWR | O_CLOEXEC | O_NONBLOCK); if (f < 0) { switch (errno) { case ENOENT: { @@ -129,8 +134,11 @@ uint64_t FileAccessUnixPipe::get_buffer(uint8_t *p_dst, uint64_t p_length) const ERR_FAIL_COND_V_MSG(fd[0] < 0, -1, "Pipe must be opened before use."); ERR_FAIL_COND_V(!p_dst && p_length > 0, -1); - uint64_t read = ::read(fd[0], p_dst, p_length); - if (read == p_length) { + ssize_t read = ::read(fd[0], p_dst, p_length); + if (read == -1) { + last_error = ERR_FILE_CANT_READ; + read = 0; + } else if (read != (ssize_t)p_length) { last_error = ERR_FILE_CANT_READ; } else { last_error = OK; diff --git a/drivers/unix/file_access_unix_pipe.h b/drivers/unix/file_access_unix_pipe.h index 19acdb5a375..1a4199f2399 100644 --- a/drivers/unix/file_access_unix_pipe.h +++ b/drivers/unix/file_access_unix_pipe.h @@ -50,7 +50,7 @@ class FileAccessUnixPipe : public FileAccess { void _close(); public: - Error open_existing(int p_rfd, int p_wfd); + Error open_existing(int p_rfd, int p_wfd, bool p_blocking); virtual Error open_internal(const String &p_path, int p_mode_flags) override; ///< open a file virtual bool is_open() const override; ///< true when file is open diff --git a/drivers/unix/os_unix.cpp b/drivers/unix/os_unix.cpp index ce2553456d9..8a9b1300689 100644 --- a/drivers/unix/os_unix.cpp +++ b/drivers/unix/os_unix.cpp @@ -493,7 +493,7 @@ Dictionary OS_Unix::get_memory_info() const { return meminfo; } -Dictionary OS_Unix::execute_with_pipe(const String &p_path, const List &p_arguments) { +Dictionary OS_Unix::execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking) { #define CLEAN_PIPES \ if (pipe_in[0] >= 0) { \ ::close(pipe_in[0]); \ @@ -578,11 +578,11 @@ Dictionary OS_Unix::execute_with_pipe(const String &p_path, const List & Ref main_pipe; main_pipe.instantiate(); - main_pipe->open_existing(pipe_out[0], pipe_in[1]); + main_pipe->open_existing(pipe_out[0], pipe_in[1], p_blocking); Ref err_pipe; err_pipe.instantiate(); - err_pipe->open_existing(pipe_err[0], 0); + err_pipe->open_existing(pipe_err[0], 0, p_blocking); ProcessInfo pi; process_map_mutex.lock(); diff --git a/drivers/unix/os_unix.h b/drivers/unix/os_unix.h index df269a59d32..3add5df055e 100644 --- a/drivers/unix/os_unix.h +++ b/drivers/unix/os_unix.h @@ -83,7 +83,7 @@ public: virtual Dictionary get_memory_info() const override; virtual Error execute(const String &p_path, const List &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr, bool p_open_console = false) override; - virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments) override; + virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking = true) override; virtual Error create_process(const String &p_path, const List &p_arguments, ProcessID *r_child_id = nullptr, bool p_open_console = false) override; virtual Error kill(const ProcessID &p_pid) override; virtual int get_process_id() const override; diff --git a/drivers/windows/file_access_windows_pipe.cpp b/drivers/windows/file_access_windows_pipe.cpp index 0c953b14aad..9bf0f4d8527 100644 --- a/drivers/windows/file_access_windows_pipe.cpp +++ b/drivers/windows/file_access_windows_pipe.cpp @@ -35,7 +35,7 @@ #include "core/os/os.h" #include "core/string/print_string.h" -Error FileAccessWindowsPipe::open_existing(HANDLE p_rfd, HANDLE p_wfd) { +Error FileAccessWindowsPipe::open_existing(HANDLE p_rfd, HANDLE p_wfd, bool p_blocking) { // Open pipe using handles created by CreatePipe(rfd, wfd, NULL, 4096) call in the OS.execute_with_pipe. _close(); @@ -44,6 +44,12 @@ Error FileAccessWindowsPipe::open_existing(HANDLE p_rfd, HANDLE p_wfd) { fd[0] = p_rfd; fd[1] = p_wfd; + if (!p_blocking) { + DWORD mode = PIPE_READMODE_BYTE | PIPE_NOWAIT; + SetNamedPipeHandleState(fd[0], &mode, nullptr, nullptr); + SetNamedPipeHandleState(fd[1], &mode, nullptr, nullptr); + } + last_error = OK; return OK; } @@ -58,7 +64,7 @@ Error FileAccessWindowsPipe::open_internal(const String &p_path, int p_mode_flag HANDLE h = CreateFileW((LPCWSTR)path.utf16().get_data(), GENERIC_READ | GENERIC_WRITE, 0, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (h == INVALID_HANDLE_VALUE) { - h = CreateNamedPipeW((LPCWSTR)path.utf16().get_data(), PIPE_ACCESS_DUPLEX, PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 4096, 4096, 0, nullptr); + h = CreateNamedPipeW((LPCWSTR)path.utf16().get_data(), PIPE_ACCESS_DUPLEX, PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_NOWAIT, 1, 4096, 4096, 0, nullptr); if (h == INVALID_HANDLE_VALUE) { last_error = ERR_FILE_CANT_OPEN; return last_error; @@ -100,7 +106,7 @@ uint64_t FileAccessWindowsPipe::get_buffer(uint8_t *p_dst, uint64_t p_length) co ERR_FAIL_COND_V_MSG(fd[0] == 0, -1, "Pipe must be opened before use."); ERR_FAIL_COND_V(!p_dst && p_length > 0, -1); - DWORD read = -1; + DWORD read = 0; if (!ReadFile(fd[0], p_dst, p_length, &read, nullptr) || read != p_length) { last_error = ERR_FILE_CANT_READ; } else { diff --git a/drivers/windows/file_access_windows_pipe.h b/drivers/windows/file_access_windows_pipe.h index 4e9bd036ae5..1eb3c6ef2fc 100644 --- a/drivers/windows/file_access_windows_pipe.h +++ b/drivers/windows/file_access_windows_pipe.h @@ -49,7 +49,7 @@ class FileAccessWindowsPipe : public FileAccess { void _close(); public: - Error open_existing(HANDLE p_rfd, HANDLE p_wfd); + Error open_existing(HANDLE p_rfd, HANDLE p_wfd, bool p_blocking); virtual Error open_internal(const String &p_path, int p_mode_flags) override; ///< open a file virtual bool is_open() const override; ///< true when file is open diff --git a/misc/extension_api_validation/4.3-stable.expected b/misc/extension_api_validation/4.3-stable.expected index 882f96a0dc5..ad77e30e11c 100644 --- a/misc/extension_api_validation/4.3-stable.expected +++ b/misc/extension_api_validation/4.3-stable.expected @@ -73,3 +73,10 @@ Validate extension JSON: Error: Field 'classes/EditorInterface/methods/popup_nod Validate extension JSON: Error: Field 'classes/EditorInterface/methods/popup_property_selector/arguments': size changed value in new API, from 3 to 4. Added optional argument to popup_property_selector and popup_node_selector to specify the current value. + + +GH-94434 +-------- +Validate extension JSON: Error: Field 'classes/OS/methods/execute_with_pipe/arguments': size changed value in new API, from 2 to 3. + +Optional argument added. Compatibility method registered. diff --git a/platform/web/os_web.cpp b/platform/web/os_web.cpp index ef8f90421ba..51facbaa845 100644 --- a/platform/web/os_web.cpp +++ b/platform/web/os_web.cpp @@ -105,7 +105,7 @@ Error OS_Web::execute(const String &p_path, const List &p_arguments, Str return create_process(p_path, p_arguments); } -Dictionary OS_Web::execute_with_pipe(const String &p_path, const List &p_arguments) { +Dictionary OS_Web::execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking) { ERR_FAIL_V_MSG(Dictionary(), "OS::execute_with_pipe is not available on the Web platform."); } diff --git a/platform/web/os_web.h b/platform/web/os_web.h index 55a5fcc6c62..1ddb745965a 100644 --- a/platform/web/os_web.h +++ b/platform/web/os_web.h @@ -80,7 +80,7 @@ public: bool main_loop_iterate(); Error execute(const String &p_path, const List &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr, bool p_open_console = false) override; - Dictionary execute_with_pipe(const String &p_path, const List &p_arguments) override; + Dictionary execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking = true) override; Error create_process(const String &p_path, const List &p_arguments, ProcessID *r_child_id = nullptr, bool p_open_console = false) override; Error kill(const ProcessID &p_pid) override; int get_process_id() const override; diff --git a/platform/windows/os_windows.cpp b/platform/windows/os_windows.cpp index bcc6a64671c..f9c636a4a61 100644 --- a/platform/windows/os_windows.cpp +++ b/platform/windows/os_windows.cpp @@ -878,7 +878,7 @@ Dictionary OS_Windows::get_memory_info() const { return meminfo; } -Dictionary OS_Windows::execute_with_pipe(const String &p_path, const List &p_arguments) { +Dictionary OS_Windows::execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking) { #define CLEAN_PIPES \ if (pipe_in[0] != 0) { \ CloseHandle(pipe_in[0]); \ @@ -977,11 +977,11 @@ Dictionary OS_Windows::execute_with_pipe(const String &p_path, const List main_pipe; main_pipe.instantiate(); - main_pipe->open_existing(pipe_out[0], pipe_in[1]); + main_pipe->open_existing(pipe_out[0], pipe_in[1], p_blocking); Ref err_pipe; err_pipe.instantiate(); - err_pipe->open_existing(pipe_err[0], 0); + err_pipe->open_existing(pipe_err[0], 0, p_blocking); ret["stdio"] = main_pipe; ret["stderr"] = err_pipe; diff --git a/platform/windows/os_windows.h b/platform/windows/os_windows.h index 9c7b98d7fdd..4f9bc049ee5 100644 --- a/platform/windows/os_windows.h +++ b/platform/windows/os_windows.h @@ -188,7 +188,7 @@ public: virtual Dictionary get_memory_info() const override; virtual Error execute(const String &p_path, const List &p_arguments, String *r_pipe = nullptr, int *r_exitcode = nullptr, bool read_stderr = false, Mutex *p_pipe_mutex = nullptr, bool p_open_console = false) override; - virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments) override; + virtual Dictionary execute_with_pipe(const String &p_path, const List &p_arguments, bool p_blocking = true) override; virtual Error create_process(const String &p_path, const List &p_arguments, ProcessID *r_child_id = nullptr, bool p_open_console = false) override; virtual Error kill(const ProcessID &p_pid) override; virtual int get_process_id() const override;