From 08e269d9c2a7faaa08b27e0fb63d50c20529a5c2 Mon Sep 17 00:00:00 2001 From: yum Date: Wed, 22 Feb 2023 21:46:48 -0800 Subject: Finish reimplementing synchronous process layer Use raw WIN32 APIs to launch processes instead of wxProcess. This enables spawning processes from arbitrary thread contexts, such as std::async or std::thread. In the future, this layer should be redone to support streaming output. * TODO: update setting path. This is almost certainly broken for users without git installed. Test in VM! --- GUI/GUI/GUI/PythonWrapper.cpp | 123 ++++++++++++++++++++++++++++++++---------- GUI/GUI/GUI/WhisperCPP.cpp | 10 ---- 2 files changed, 95 insertions(+), 38 deletions(-) (limited to 'GUI') diff --git a/GUI/GUI/GUI/PythonWrapper.cpp b/GUI/GUI/GUI/PythonWrapper.cpp index c90520d..0f835e9 100644 --- a/GUI/GUI/GUI/PythonWrapper.cpp +++ b/GUI/GUI/GUI/PythonWrapper.cpp @@ -41,7 +41,6 @@ wxProcess* PythonWrapper::InvokeAsyncWithArgs(std::vector&& args, } auto *p = new PythonProcess(std::move(exit_callback)); - // TODO(yum) we should hide the console & stream output to a friendlier interface int pid = wxExecute(cmd_oss.str(), wxEXEC_ASYNC, p); if (!pid) { @@ -52,10 +51,27 @@ wxProcess* PythonWrapper::InvokeAsyncWithArgs(std::vector&& args, return p; } +std::string GetWin32ErrMsg() { + DWORD error = GetLastError(); + LPSTR err_msg = nullptr; + FormatMessageA( + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, + error, + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + (LPSTR)&err_msg, + 0, + NULL + ); + ScopeGuard err_msg_cleanup([&]() { LocalFree(err_msg); }); + return std::to_string(error) + ": " + err_msg; +} + bool PythonWrapper::InvokeCommandWithArgs( const std::string& cmd, std::vector&& args, std::string* py_stdout, std::string* py_stderr) { + std::ostringstream cmd_oss; cmd_oss << cmd; for (const auto& arg : args) { @@ -64,66 +80,96 @@ bool PythonWrapper::InvokeCommandWithArgs( HANDLE stdout_read{}; HANDLE stdout_write{}; - SECURITY_ATTRIBUTES sec_attr{}; - sec_attr.nLength = sizeof(sec_attr); - sec_attr.bInheritHandle = TRUE; - if (!CreatePipe(&stdout_read, &stdout_write, &sec_attr, 0)) { + SECURITY_ATTRIBUTES stdout_sec_attr{}; + stdout_sec_attr.nLength = sizeof(stdout_sec_attr); + stdout_sec_attr.bInheritHandle = TRUE; + if (!CreatePipe(&stdout_read, &stdout_write, &stdout_sec_attr, 0)) { if (py_stderr) { std::ostringstream err_oss; err_oss << "Error while executing python command \"" << cmd_oss.str() - << "\": Failed to create stdout pipe" << std::endl; + << "\": Failed to create stdout pipe: " << GetWin32ErrMsg() << std::endl; *py_stderr = err_oss.str(); } return false; } ScopeGuard stdout_cleanup([&]() { - CloseHandle(stdout_read); - CloseHandle(stdout_write); + if (stdout_read) { + CloseHandle(stdout_read); + } + if (stdout_write) { + CloseHandle(stdout_write); + } }); + SetHandleInformation(stdout_read, HANDLE_FLAG_INHERIT, 0); HANDLE stderr_read{}; HANDLE stderr_write{}; - if (!CreatePipe(&stderr_read, &stderr_write, &sec_attr, 0)) { + SECURITY_ATTRIBUTES stderr_sec_attr{}; + stderr_sec_attr.nLength = sizeof(stderr_sec_attr); + stderr_sec_attr.bInheritHandle = TRUE; + + if (!CreatePipe(&stderr_read, &stderr_write, &stderr_sec_attr, 0)) { if (py_stderr) { std::ostringstream err_oss; err_oss << "Error while executing python command \"" << cmd_oss.str() - << "\": Failed to create stderr pipe" << std::endl; + << "\": Failed to create stderr pipe: " << GetWin32ErrMsg() << std::endl; *py_stderr = err_oss.str(); } return false; } ScopeGuard stderr_cleanup([&]() { - CloseHandle(stderr_read); - CloseHandle(stderr_write); + if (stderr_read) { + CloseHandle(stderr_read); + } + if (stderr_write) { + CloseHandle(stderr_write); + } }); + SetHandleInformation(stderr_read, HANDLE_FLAG_INHERIT, 0); STARTUPINFOA si{}; si.cb = sizeof(si); si.hStdOutput = stdout_write; si.hStdError = stderr_write; + si.dwFlags |= STARTF_USESTDHANDLES; + si.dwFlags |= STARTF_USESHOWWINDOW; + si.wShowWindow = SW_HIDE; PROCESS_INFORMATION pi{}; - std::string path = "Resources/PortableGit/bin"; - std::string env = "PATH=" + path; - //std::string env; + std::string env; - //std::string tmp_cmd = "Get-ChildItem"; + { + std::vector buf(4096, 0); + DWORD len = GetEnvironmentVariableA("PATH", buf.data(), buf.size() - 1); + if (len > 0) { + env = std::string("PATH=") + buf.data(); + } + else { + std::ostringstream err_oss; + err_oss << "Error while executing python command \"" << cmd_oss.str() + << "\": Failed to get PATH env variable: " << GetWin32ErrMsg() << std::endl; + *py_stderr = err_oss.str(); + return false; + } + // TODO(yum) add git to PATH + } std::string cmd_str = cmd_oss.str(); if (!CreateProcessA(NULL, // application name cmd_str.data(), - //tmp_cmd.data(), NULL, // process attributes NULL, // thread attributes - FALSE, // whether to inherit parent's handles + TRUE, // whether to inherit parent's handles 0, // creation flags - env.data(), - NULL, // current directory (use parent's) + //env.data(), + nullptr, // env + std::filesystem::current_path().string().c_str(), // current directory &si, &pi)) { if (py_stderr) { std::ostringstream err_oss; + err_oss << "Error while executing python command \"" << cmd_oss.str() - << "\": Failed to launch process" << std::endl; + << "\": Failed to launch process: " << GetWin32ErrMsg(); *py_stderr = err_oss.str(); } return false; @@ -133,26 +179,47 @@ bool PythonWrapper::InvokeCommandWithArgs( CloseHandle(pi.hThread); }); + WaitForSingleObject(pi.hProcess, INFINITE); std::ostringstream stdout_oss, stderr_oss; + + DWORD exit_code; + if (!GetExitCodeProcess(pi.hProcess, &exit_code)) { + stderr_oss << "Failed to get exit code!\n"; + } + if (exit_code != 0) { + stderr_oss << "Command exited with code " << exit_code << ": " + << GetWin32ErrMsg() << std::endl; + } + + WaitForSingleObject(pi.hProcess, INFINITE); + + // Close write ends of pipes. If we don't do this, the last read will block forever. + CloseHandle(stdout_write); + stdout_write = 0; + CloseHandle(stderr_write); + stderr_write = 0; + std::vector buf(4096, 0); DWORD bytes_read = 0; - while (ReadFile(si.hStdOutput, buf.data(), buf.size(), &bytes_read, NULL) && bytes_read > 0) { + while (ReadFile(stdout_read, buf.data(), buf.size(), &bytes_read, NULL) && bytes_read > 0) { stdout_oss << std::string(buf.data(), bytes_read); } + if (bytes_read < 0) { + stdout_oss << "Reading stdout failed: " << GetWin32ErrMsg(); + } bytes_read = 0; - while (ReadFile(si.hStdError, buf.data(), buf.size(), &bytes_read, NULL) && bytes_read > 0) { + while (ReadFile(stderr_read, buf.data(), buf.size(), &bytes_read, NULL) && bytes_read > 0) { stderr_oss << std::string(buf.data(), bytes_read); } - - WaitForSingleObject(pi.hProcess, INFINITE); - - // TODO(yum) retrieve exit code + if (bytes_read < 0) { + stderr_oss << "Reading stderr failed: " << GetWin32ErrMsg(); + } *py_stdout = stdout_oss.str(); if (py_stderr) { *py_stderr = stderr_oss.str(); } - return true; + return exit_code == 0; } bool PythonWrapper::InvokeWithArgs(std::vector&& args, diff --git a/GUI/GUI/GUI/WhisperCPP.cpp b/GUI/GUI/GUI/WhisperCPP.cpp index 362bad0..2bb5b34 100644 --- a/GUI/GUI/GUI/WhisperCPP.cpp +++ b/GUI/GUI/GUI/WhisperCPP.cpp @@ -247,7 +247,6 @@ void WhisperCPP::Start(const AppConfig& c) { if (!InstallDependencies()) { return; } -#if 1 std::filesystem::path model_path = "Resources/Models"; model_path /= c.whisper_model; @@ -360,15 +359,6 @@ void WhisperCPP::Start(const AppConfig& c) { Log(out_, "Capture failed: {}\n", hresultToString(err)); return; } -#else - while (run_) { - static int i = 0; - if (++i % 100 == 0) { - Log(out_, "Spin {}\n", i); - } - std::this_thread::sleep_for(std::chrono::milliseconds(10)); - } -#endif Log(out_, "Exit transcription engine\n"); }); -- cgit v1.2.3