From e2969d30fddb6a05fba99e69b86894c799d36877 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 01:36:40 -0300 Subject: [PATCH 01/13] Compiles the agent as Console App when building the MSIX --- msix/agent/agent.targets | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/msix/agent/agent.targets b/msix/agent/agent.targets index f4f6c64bd..c21292473 100644 --- a/msix/agent/agent.targets +++ b/msix/agent/agent.targets @@ -3,9 +3,6 @@ $(MSBuildThisFileDirectory)..\..\windows-agent\ $(GoAppRoot)cmd\ubuntu-pro-agent\ - - - -ldflags -H=windowsgui -tags=server_mocks @@ -33,7 +30,7 @@ - + From 82c9b05d2a18e3a7b0ef47980e07edce0c958468 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 01:49:05 -0300 Subject: [PATCH 02/13] Adds the launcher vcxproj to the msix solution --- msix/msix.sln | 8 +- msix/ubuntu-pro-agent-launcher/main.cpp | 8 ++ .../ubuntu-pro-agent-launcher.vcxproj | 80 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 msix/ubuntu-pro-agent-launcher/main.cpp create mode 100644 msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj diff --git a/msix/msix.sln b/msix/msix.sln index bb814a502..41b4b0483 100644 --- a/msix/msix.sln +++ b/msix/msix.sln @@ -7,10 +7,12 @@ Project("{C7167F0D-BC9F-4E6E-AFE1-012C56B48DB5}") = "UbuntuProForWindows", "Ubun EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ubuntupro", "gui\gui.vcxproj", "{89935278-02C9-414F-8DB5-47A465BC1F2B}" EndProject -Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "agent", "agent\agent.vcxproj", "{6427BB31-C4BE-4585-A090-10EFDFB06B04}" +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ubuntu-pro-agent", "agent\agent.vcxproj", "{6427BB31-C4BE-4585-A090-10EFDFB06B04}" EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "storeapi", "storeapi\storeapi.vcxproj", "{4EE3B168-C58D-4AE5-A259-1B5A04C018DE}" EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ubuntu-pro-agent-launcher", "ubuntu-pro-agent-launcher\ubuntu-pro-agent-launcher.vcxproj", "{EB330327-E1D2-4AB7-9980-FA2E56BF308B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|x64 = Debug|x64 @@ -35,6 +37,10 @@ Global {4EE3B168-C58D-4AE5-A259-1B5A04C018DE}.Debug|x64.Build.0 = Debug|x64 {4EE3B168-C58D-4AE5-A259-1B5A04C018DE}.Release|x64.ActiveCfg = Release|x64 {4EE3B168-C58D-4AE5-A259-1B5A04C018DE}.Release|x64.Build.0 = Release|x64 + {EB330327-E1D2-4AB7-9980-FA2E56BF308B}.Debug|x64.ActiveCfg = Debug|x64 + {EB330327-E1D2-4AB7-9980-FA2E56BF308B}.Debug|x64.Build.0 = Debug|x64 + {EB330327-E1D2-4AB7-9980-FA2E56BF308B}.Release|x64.ActiveCfg = Release|x64 + {EB330327-E1D2-4AB7-9980-FA2E56BF308B}.Release|x64.Build.0 = Release|x64 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/msix/ubuntu-pro-agent-launcher/main.cpp b/msix/ubuntu-pro-agent-launcher/main.cpp new file mode 100644 index 000000000..f9322a483 --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/main.cpp @@ -0,0 +1,8 @@ +/// A Windows Application that creates an invisible pseudo-console to host the ubuntu-pro-agent.exe; +#include + +int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, + PWSTR pCmdLine, int nCmdShow) +{ + return 0; +} diff --git a/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj new file mode 100644 index 000000000..4c82db67b --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj @@ -0,0 +1,80 @@ + + + + + Debug + x64 + + + Release + x64 + + + + 17.0 + Win32Proj + {eb330327-e1d2-4ab7-9980-fa2e56bf308b} + ubuntuproagentlauncher + 10.0 + + + + Application + true + v143 + Unicode + + + Application + false + v143 + true + Unicode + + + + + + + + + + + + + + + + Level3 + true + _DEBUG;_CONSOLE;%(PreprocessorDefinitions) + true + + + Windows + true + + + + + Level3 + true + true + true + NDEBUG;_CONSOLE;%(PreprocessorDefinitions) + true + + + Windows + true + true + true + + + + + + + + + From 34e06b0332566dba62ae6ebf435503ad6da827ea Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 01:50:10 -0300 Subject: [PATCH 03/13] Refers to the launcher in the agent's vcxproj That allows enforcing the final location in the package Both agent and launcher will stay in the same directory. --- msix/agent/agent.vcxproj | 3 +++ .../ubuntu-pro-agent-launcher.vcxproj | 2 ++ 2 files changed, 5 insertions(+) diff --git a/msix/agent/agent.vcxproj b/msix/agent/agent.vcxproj index f8eca2698..74a46dc51 100644 --- a/msix/agent/agent.vcxproj +++ b/msix/agent/agent.vcxproj @@ -123,6 +123,9 @@ {4ee3b168-c58d-4ae5-a259-1b5a04c018de} + + {eb330327-e1d2-4ab7-9980-fa2e56bf308b} + diff --git a/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj index 4c82db67b..0a10a65c1 100644 --- a/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj +++ b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj @@ -49,6 +49,7 @@ true _DEBUG;_CONSOLE;%(PreprocessorDefinitions) true + stdcpp20 Windows @@ -63,6 +64,7 @@ true NDEBUG;_CONSOLE;%(PreprocessorDefinitions) true + stdcpp20 Windows From 93d65090b33970486ad85563af89c583884a45e4 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 23:28:06 -0300 Subject: [PATCH 04/13] The main I want to write GUI applications rely on some sort of event loop to mediate the unpredictable timing user input will happen. The Windows OS models this by means of the window message loop and the WndProc function that says how the window responds to messages it receives. We want to wait on ReadFile to drain the console pipes, so the child process won't block on the next printf after the pipe becomes full. We also want to wait on the agent process so we can quit if it quits. We also want to respond to messages sent by the OS (such as WM_QUIT). Threads without actual windows still have the message loop. We can combine the window approach with some waiting functions and create a custom message loop that allows our thread to sleep while we don't receive any relevant input neither on the thread queue, nor on the console pipe or the agent process termination. Creating the pseudo-console should be declarative. Starting the agent process under the pseudo-console should be a function call. Reacting to the agent's process termination or new input from the console pipe should resemble reactive programming by passing functions. I don't want to see tons of Win32 in my main() I hope you enjoy this small spec. :) --- msix/ubuntu-pro-agent-launcher/main.cpp | 85 +++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/msix/ubuntu-pro-agent-launcher/main.cpp b/msix/ubuntu-pro-agent-launcher/main.cpp index f9322a483..29974d345 100644 --- a/msix/ubuntu-pro-agent-launcher/main.cpp +++ b/msix/ubuntu-pro-agent-launcher/main.cpp @@ -1,8 +1,83 @@ -/// A Windows Application that creates an invisible pseudo-console to host the ubuntu-pro-agent.exe; +/// A Windows Application that creates an invisible pseudo-console to host the +/// ubuntu-pro-agent.exe; #include -int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, - PWSTR pCmdLine, int nCmdShow) -{ - return 0; +#include +#include +#include +#include +#include + +#include "console.hpp" +#include "error.hpp" + +std::filesystem::path const& logPath() { + static std::filesystem::path localAppDataPath = + up4w::MakePathRelativeToEnvDir( + L"\\Ubuntu Pro\\ubuntu-pro-agent-launcher.log", L"LOCALAPPDATA"); + return localAppDataPath; +} + +std::filesystem::path thisBinaryDir() { + wchar_t binaryPath[MAX_PATH]; + DWORD fnLength = GetModuleFileName(nullptr, binaryPath, MAX_PATH); + if (fnLength == 0) { + return std::filesystem::path(); + } + std::filesystem::path exePath{std::wstring_view{binaryPath, fnLength}}; + exePath.remove_filename(); + + return exePath; +} + +int WINAPI wWinMain(HINSTANCE, HINSTANCE, PWSTR pCmdLine, int) try { + // setup the app: pipes and console + up4w::PseudoConsole console{{.X = 80, .Y = 80}}; + + // start the child process + auto agent = thisBinaryDir() / L"ubuntu-pro-agent.exe"; + auto p = console.StartProcess(std::format(L"{} {}", agent.c_str(), pCmdLine)); + + // setup the event loop with listeners. + up4w::EventLoop ev{{ + p.hProcess, + [](HANDLE process) { + DWORD exitCode = 0; + GetExitCodeProcess(process, &exitCode); + return exitCode; + }, + }, + { + console.GetReadHandle(), + [](HANDLE read) { + std::array buffer{}; + DWORD bytesRead = 0; + ReadFile(read, &buffer[0], + static_cast(buffer.size() - 1), + &bytesRead, nullptr); + return std::nullopt; + }, + }}; + + // dispatch the event loop + return ev.Run(); + + // log errors, if any. +} catch (up4w::hresult_exception const& err) { + std::filesystem::path const& localAppDataPath = logPath(); + if (localAppDataPath.empty()) { + return 1; + } + + auto msg = std::format("{}\n\t{}", err.message().c_str(), err.where()); + up4w::LogSingleShot(localAppDataPath, msg); + return 2; +} catch (std::exception const& err) { + std::filesystem::path const& localAppDataPath = logPath(); + if (localAppDataPath.empty()) { + return 1; + } + + up4w::LogSingleShot(localAppDataPath, err.what()); + return 3; } From b11162c9fbdd7e6617f581b739df272de3f18915 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 23:47:03 -0300 Subject: [PATCH 05/13] The desired pseudo console API --- msix/ubuntu-pro-agent-launcher/console.hpp | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 msix/ubuntu-pro-agent-launcher/console.hpp diff --git a/msix/ubuntu-pro-agent-launcher/console.hpp b/msix/ubuntu-pro-agent-launcher/console.hpp new file mode 100644 index 000000000..06b64b65d --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/console.hpp @@ -0,0 +1,43 @@ +#pragma once +#include + +#include + +namespace up4w { +// An RAII wrapper around the PROCESS_INFORMATION structure to ease preventing +// HANDLE leaks. +struct Process : PROCESS_INFORMATION { + ~Process() { + if (hThread != nullptr && hThread != INVALID_HANDLE_VALUE) { + CloseHandle(hThread); + } + if (hProcess != nullptr && hProcess != INVALID_HANDLE_VALUE) { + CloseHandle(hProcess); + } + } +}; + +// An abstraction on top of the pseudo-console device that prevents leaking +// HANDLEs and makes it easier to start processes under itself. +class PseudoConsole { + HANDLE hInRead = nullptr; + HANDLE hInWrite = nullptr; + HANDLE hOutRead = nullptr; + HANDLE hOutWrite = nullptr; + + HPCON hDevice; + + public: + /// Constructs a new pseudo-console with the specified [dimensions]. + explicit PseudoConsole(COORD dimensions); + + HANDLE GetReadHandle() const { return hOutRead; } + + /// Starts a child process under this pseudo-console by running the fully + /// specified [commandLine]. + Process StartProcess(std::wstring commandLine); + + ~PseudoConsole(); +}; + +} // namespace up4w From 813df81cab8f65c0fd0f68542a0430d5717acfde Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 23:53:30 -0300 Subject: [PATCH 06/13] Making HRESULT into a usable exception type Leveraging FormatMessage function to generate a string message out of the system's catalog, based on the HRESULT value. Plus some sugar with std::source_location. That will come handy in avoiding freeing memory and closing handles in multiple code paths. A word on `unique_string`: Because FormatMessage allocates a buffer and expects the caller to later free it using Windows Heap functions this small wrapper based on std::unique_ptr makes it very handy to deal with such confusing allocate/deallocate scenarios. Logging: we don't do it everywhere, this is a very tiny application. But if an exception occur, than we should. Thus, LogSingleShot(): opens a file, writes to it and closes it. In a single shot. Not the best performance, but meant to be called just once, before quitting. --- msix/ubuntu-pro-agent-launcher/error.hpp | 66 ++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 msix/ubuntu-pro-agent-launcher/error.hpp diff --git a/msix/ubuntu-pro-agent-launcher/error.hpp b/msix/ubuntu-pro-agent-launcher/error.hpp new file mode 100644 index 000000000..0693b3d0a --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/error.hpp @@ -0,0 +1,66 @@ +#pragma once +#include + +#include +#include +#include +#include + +namespace up4w { +// A small RAII wrapper around Win32 heap allocated strings. +class unique_string { + static void string_buffer_deleter(char* buffer) { + if (buffer) { + HeapFree(GetProcessHeap(), 0, buffer); + } + } + using unique_str = std::unique_ptr; + unique_str buffer_; + + public: + const char* c_str() { return buffer_.get(); } + explicit unique_string(char* buffer) + : buffer_{buffer, &string_buffer_deleter} {} +}; + +/// Wraps Windows HRESULT into something that resembles a std::exception +class hresult_exception { + HRESULT value; + std::source_location loc_; + + public: + unique_string message() const { + char* buffer = nullptr; + ::FormatMessageA( + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, nullptr, + // Due the FORMAT_MESSAGE_ALLOCATE_BUFFER flag, what should be a char* + // (the buffer variable) has to be treated as char**, even though it's + // passed as char*. See + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessagea + value, 0, (LPSTR)&buffer, 0, nullptr); + return unique_string{buffer}; + } + std::string where() const { + return std::format("{}: {} ({})", loc_.file_name(), loc_.line(), + loc_.function_name()); + } + + explicit hresult_exception(HRESULT value, std::source_location location = + std::source_location::current()) + : value(value), loc_{location} {} + hresult_exception(const hresult_exception& other) = default; + hresult_exception(hresult_exception&& other) noexcept = default; + ~hresult_exception() = default; +}; + +/// Computes the absolute path resulting of joining the [destination] into the +/// value of the environment variable [envDir]. Returns empty string if the +/// environment variable is undefined. +std::wstring MakePathRelativeToEnvDir(std::wstring_view destination, + std::wstring_view envDir); + +// Opens the log file, writes the message with a timestamp and closes it. +void LogSingleShot(std::filesystem::path const& logFilePath, + std::string_view message); + +} // namespace up4w From 26479c8d201b77cdf52d682559883dddb7a1f379 Mon Sep 17 00:00:00 2001 From: Carlos Date: Mon, 30 Oct 2023 23:59:55 -0300 Subject: [PATCH 07/13] Implements the error handling helpers --- msix/ubuntu-pro-agent-launcher/error.cpp | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 msix/ubuntu-pro-agent-launcher/error.cpp diff --git a/msix/ubuntu-pro-agent-launcher/error.cpp b/msix/ubuntu-pro-agent-launcher/error.cpp new file mode 100644 index 000000000..c040ecc94 --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/error.cpp @@ -0,0 +1,33 @@ +#include "error.hpp" +#include +#include + +namespace up4w { +std::wstring MakePathRelativeToEnvDir(std::wstring_view destination, + std::wstring_view envDir) { + std::wstring resultPath{}; + resultPath.resize(MAX_PATH); + + auto truncatedLength = + static_cast(resultPath.capacity() - destination.size() - 1); + + auto length = + GetEnvironmentVariable(envDir.data(), resultPath.data(), truncatedLength); + if (length == 0) { + return {}; + } + + resultPath.insert(length, destination.data()); + return resultPath; +} + +void LogSingleShot(std::filesystem::path const& logFilePath, + std::string_view message) { + auto const time = + std::chrono::current_zone()->to_local(std::chrono::system_clock::now()); + + std::ofstream logfile{logFilePath}; + logfile << std::format("{:%Y-%m-%d %T}: {}\n", time, message); +} + +} // namespace up4w From 690082222e37a6bc97901a26003af5d489a205df Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:01:17 -0300 Subject: [PATCH 08/13] Construct and destruct pseudo consoles Destructor is just a bunch of checked CloseHandle calls and the most important ClosePseudoConsole call. Construction is more interesting: creates anonymous pipes to serve as the console stdio HANDLEs. The console geometry seems irrelevant for our use case, but I opted by passing it for further exploration. --- msix/ubuntu-pro-agent-launcher/console.cpp | 40 +++++++++++++++++++ .../ubuntu-pro-agent-launcher.vcxproj | 6 +++ 2 files changed, 46 insertions(+) create mode 100644 msix/ubuntu-pro-agent-launcher/console.cpp diff --git a/msix/ubuntu-pro-agent-launcher/console.cpp b/msix/ubuntu-pro-agent-launcher/console.cpp new file mode 100644 index 000000000..459e602d1 --- /dev/null +++ b/msix/ubuntu-pro-agent-launcher/console.cpp @@ -0,0 +1,40 @@ +#include "console.hpp" + +#include "error.hpp" + +namespace up4w { +PseudoConsole ::~PseudoConsole() { + if (hInRead != nullptr && hInRead != INVALID_HANDLE_VALUE) { + CloseHandle(hInRead); + } + if (hInWrite != nullptr && hInWrite != INVALID_HANDLE_VALUE) { + CloseHandle(hInWrite); + } + if (hOutRead != nullptr && hOutRead != INVALID_HANDLE_VALUE) { + CloseHandle(hOutRead); + } + if (hOutWrite != nullptr && hOutWrite != INVALID_HANDLE_VALUE) { + CloseHandle(hOutWrite); + } + if (hDevice != nullptr && hDevice != INVALID_HANDLE_VALUE) { + ClosePseudoConsole(hDevice); + } +} +PseudoConsole::PseudoConsole(COORD coordinates) { + SECURITY_ATTRIBUTES sa{sizeof(SECURITY_ATTRIBUTES), nullptr, true}; + if (!CreatePipe(&hInRead, &hInWrite, &sa, 0)) { + throw hresult_exception{HRESULT_FROM_WIN32(GetLastError())}; + } + + if (!CreatePipe(&hOutRead, &hOutWrite, &sa, 0)) { + throw hresult_exception{HRESULT_FROM_WIN32(GetLastError())}; + } + + if (auto hr = + CreatePseudoConsole(coordinates, hInRead, hOutWrite, 0, &hDevice); + FAILED(hr)) { + throw hresult_exception{hr}; + } +} + +} // namespace up4w diff --git a/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj index 0a10a65c1..fbaeeaf8a 100644 --- a/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj +++ b/msix/ubuntu-pro-agent-launcher/ubuntu-pro-agent-launcher.vcxproj @@ -74,8 +74,14 @@ + + + + + + From 7fce844c62d6b3c1c5d891f333de62128e7ed9ad Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:04:48 -0300 Subject: [PATCH 09/13] Implements StartProcess By far the most complicated part of this feature. Starting a child process under a pseudo console requires: 1. a STARTUPINFOEX structure filled with the handles the pseudo console uses for stdio 2. a LPPROC_THREAD_ATTRIBUTE_LIST containing the key PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE mapped to the HPCON (pseudo-console device handle). This list has to be allocated in the heap, since the size of its elements is hidden from the application code. We need to try to initialize it first to fail deterministically, just to learn how many bytes needs to be allocated. Then allocate the required amount of memory and try again, which should succeed. Then we fill the key-value pair we desire by calling UpdateProcThreadAttribute. Notice that's not related to the current process or thread, but for the one we'll start. 3. Then we CreateProcessW 4. Care must be taken to avoid leaking the process/thread attribute list. Disposing it requires calling DeleteProcThreadAttributeList followed by freeing the memory. Again, the unique_ptr with custom deleter idiom comes in handy. This is scoped to the process creation, once done we no longer need such structure. Since this program is meant to start only one process and live in sync with it, we don't need to either cache that thing nor make it visible outside of this routine. --- msix/ubuntu-pro-agent-launcher/console.cpp | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/msix/ubuntu-pro-agent-launcher/console.cpp b/msix/ubuntu-pro-agent-launcher/console.cpp index 459e602d1..9405bddf2 100644 --- a/msix/ubuntu-pro-agent-launcher/console.cpp +++ b/msix/ubuntu-pro-agent-launcher/console.cpp @@ -1,5 +1,8 @@ #include "console.hpp" +#include +#include + #include "error.hpp" namespace up4w { @@ -37,4 +40,64 @@ PseudoConsole::PseudoConsole(COORD coordinates) { } } +void attr_list_deleter(PPROC_THREAD_ATTRIBUTE_LIST p) { + if (p) { + DeleteProcThreadAttributeList(p); + HeapFree(GetProcessHeap(), 0, p); + } +}; +using unique_attr_list = + std::unique_ptr, + decltype(&attr_list_deleter)>; + +/// Returns a list of attributes for process/thread creation with the +/// pseudo-console key enabled and set to [con]. +unique_attr_list PseudoConsoleProcessAttrList(HPCON con) { + PPROC_THREAD_ATTRIBUTE_LIST attrs = nullptr; + + size_t bytesRequired = 0; + InitializeProcThreadAttributeList(NULL, 1, 0, &bytesRequired); + // Allocate memory to represent the list + attrs = static_cast( + HeapAlloc(GetProcessHeap(), 0, bytesRequired)); + if (!attrs) { + throw hresult_exception{E_OUTOFMEMORY}; + } + + // Initialize the list memory location + if (!InitializeProcThreadAttributeList(attrs, 1, 0, &bytesRequired)) { + throw hresult_exception{HRESULT_FROM_WIN32(GetLastError())}; + } + + unique_attr_list result{attrs, &attr_list_deleter}; + + if (!UpdateProcThreadAttribute(attrs, 0, PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + con, sizeof(con), NULL, NULL)) { + throw hresult_exception{HRESULT_FROM_WIN32(GetLastError())}; + } + + return result; +} + +Process PseudoConsole::StartProcess(std::wstring commandLine) { + unique_attr_list attributes = PseudoConsoleProcessAttrList(hDevice); + // Prepare Startup Information structure + STARTUPINFOEX si{}; + si.StartupInfo.cb = sizeof(STARTUPINFOEX); + si.StartupInfo.hStdInput = hInRead; + si.StartupInfo.hStdOutput = hOutWrite; + si.StartupInfo.hStdError = hOutWrite; + si.StartupInfo.dwFlags = STARTF_USESTDHANDLES; + si.lpAttributeList = attributes.get(); + + Process p{0}; + if (!CreateProcessW(NULL, commandLine.data(), NULL, NULL, FALSE, + EXTENDED_STARTUPINFO_PRESENT, NULL, NULL, &si.StartupInfo, + &p)) { + throw hresult_exception{HRESULT_FROM_WIN32(GetLastError())}; + } + + return p; +} + } // namespace up4w From 7456cbcf3fab4ee416515f5f6edff68ed036a6fb Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:13:47 -0300 Subject: [PATCH 10/13] Implements the custom event loop Although we never create or show a window, this is still a GUI application Thus it's powered by the window/thread message loop I mentioned before. The need for the event loop might still not be clear enough. Let me detail it: 1. Pseudo-console stdio HANDLEs are anonymous pipes HANDLEs. 2. The child process inherits the parts it needs to implement stdio. 3. The parent (this program) holds the other end of the pipes. 4. If stdout pipe becomes full, the child process will block at the next attempt to printf. 5. Thus we need to periodically read from the pipe to drain it. 6. We can do whatever we want with the bytes we read, nothing inclusive. 7. But we must read it. 8. Calling ReadFile on an empty pipe blocks the caller until new data arrives. 9. The child process might quit without writing anything new. 10. The parent will wait forever. 11. Users could kill this program (via task manager, for example) 12. That causes the process to receive WM_QUIT (amongst other related messages) 13. We need to respond to that message (and others maybe). A lot of asynchrony. MsgWaitForMultipleObjectsEx to the rescue. Seems made for this problem. We can wait on multiple HANDLEs that the OS can interpret as synchronizeable, i.e. that has some state that can change meaningfully, such as a process that terminates or a pipe that receives new data. Not to be confused with Overlapped IO. That's a whole other world. It also waits on new messages arriving to the window or thread message queue. If configured accordingly it blocks until one of the interesting activities happen, letting us know which one, so we can handle it accordingly. The association between which HANDLE to watch and which behavior to adopt could be hardcoded in the event loop itself, but I found so beautiful it passed as a parameter at the event loop construction. Otherwise the event loop would be part of the PseudoConsole class, making it like a God. --- msix/ubuntu-pro-agent-launcher/console.cpp | 41 ++++++++++++++++++++++ msix/ubuntu-pro-agent-launcher/console.hpp | 23 ++++++++++++ 2 files changed, 64 insertions(+) diff --git a/msix/ubuntu-pro-agent-launcher/console.cpp b/msix/ubuntu-pro-agent-launcher/console.cpp index 9405bddf2..a573a03e6 100644 --- a/msix/ubuntu-pro-agent-launcher/console.cpp +++ b/msix/ubuntu-pro-agent-launcher/console.cpp @@ -100,4 +100,45 @@ Process PseudoConsole::StartProcess(std::wstring commandLine) { return p; } +void EventLoop::reserve(std::size_t size) { + handles_.reserve(size); + listeners_.reserve(size); +} + +EventLoop::EventLoop( + std::initializer_list< + std::pair(HANDLE)>>> + listeners) { + reserve(listeners.size()); + for (auto& [k, f] : listeners) { + handles_.push_back(k); + listeners_.push_back(f); + } +} + +int EventLoop::Run() { + do { + DWORD signaledIndex = MsgWaitForMultipleObjectsEx( + static_cast(handles_.size()), handles_.data(), INFINITE, + QS_ALLEVENTS, MWMO_INPUTAVAILABLE); + // none of the handles, thus the window message queue was signaled. + if (signaledIndex >= handles_.size()) { + MSG msg; + if (!GetMessage(&msg, NULL, 0, 0)) { + // WM_QUIT + return 0; + } + + TranslateMessage(&msg); + DispatchMessage(&msg); + } else { + // invoke the listener subscribed to the handle that was signaled. + if (auto done = listeners_[signaledIndex](handles_[signaledIndex]); + done.has_value()) { + return done.value(); + } + } + } while (true); +} + } // namespace up4w diff --git a/msix/ubuntu-pro-agent-launcher/console.hpp b/msix/ubuntu-pro-agent-launcher/console.hpp index 06b64b65d..8af931d17 100644 --- a/msix/ubuntu-pro-agent-launcher/console.hpp +++ b/msix/ubuntu-pro-agent-launcher/console.hpp @@ -1,7 +1,11 @@ #pragma once #include +#include +#include +#include #include +#include namespace up4w { // An RAII wrapper around the PROCESS_INFORMATION structure to ease preventing @@ -40,4 +44,23 @@ class PseudoConsole { ~PseudoConsole(); }; +/// A combination of traditional window message loop with event listening. +/// Listener functions return any integer value other than nullopt to report +/// that the event loop should exit. +class EventLoop { + std::vector handles_; + std::vector(HANDLE)>> listeners_; + void reserve(std::size_t size); + + public: + explicit EventLoop( + std::initializer_list< + std::pair(HANDLE)>>> + listeners); + + // Runs the event loop until one of the listeners return a value or a closing + // message is received in the message queue. + int Run(); +}; + } // namespace up4w From 1cd529e56376f70929bc2099a211c0be7658d448 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:24:59 -0300 Subject: [PATCH 11/13] Replaces the agent by the launcher As the fulltrust process and startup task in the appxmanifest. --- msix/UbuntuProForWindows/Package.appxmanifest | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/msix/UbuntuProForWindows/Package.appxmanifest b/msix/UbuntuProForWindows/Package.appxmanifest index b79934103..64c172b5d 100644 --- a/msix/UbuntuProForWindows/Package.appxmanifest +++ b/msix/UbuntuProForWindows/Package.appxmanifest @@ -62,14 +62,14 @@ - + From 8d8b22af5b7930886dfb5e3a42fafd1d9e4636a2 Mon Sep 17 00:00:00 2001 From: Carlos Date: Tue, 31 Oct 2023 00:26:31 -0300 Subject: [PATCH 12/13] Updates the GUI to start the launcher instead of the real agent, which is now a CLI application Also updates test code that builds the agent The GUI expects the launcher now. --- gui/packages/ubuntupro/integration_test/utils/build_agent.dart | 2 +- gui/packages/ubuntupro/lib/constants.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gui/packages/ubuntupro/integration_test/utils/build_agent.dart b/gui/packages/ubuntupro/integration_test/utils/build_agent.dart index 1aa50f305..c675cc875 100644 --- a/gui/packages/ubuntupro/integration_test/utils/build_agent.dart +++ b/gui/packages/ubuntupro/integration_test/utils/build_agent.dart @@ -7,7 +7,7 @@ import 'package:path/path.dart' as p; /// A try-catch block (on AssertionError) can still prevent process exit. Future buildAgentExe( String destination, { - String exeName = 'ubuntu-pro-agent.exe', + String exeName = 'ubuntu-pro-agent-launcher.exe', }) async { const config = 'Debug'; const platform = 'x64'; diff --git a/gui/packages/ubuntupro/lib/constants.dart b/gui/packages/ubuntupro/lib/constants.dart index 713f326ad..13e1224eb 100644 --- a/gui/packages/ubuntupro/lib/constants.dart +++ b/gui/packages/ubuntupro/lib/constants.dart @@ -11,4 +11,4 @@ const kAddrFileName = 'addr'; const kDefaultMargin = 32.0; /// The path of the agent executable relative to the msix root directory. -const kAgentRelativePath = 'agent/ubuntu-pro-agent.exe'; +const kAgentRelativePath = 'agent/ubuntu-pro-agent-launcher.exe'; From f5e7183c7263425f0e65363739945039cf8c130f Mon Sep 17 00:00:00 2001 From: Carlos Date: Sat, 4 Nov 2023 07:31:24 +0200 Subject: [PATCH 13/13] Addresses PR review comments squashing commits to keep the history clean. Deletes copy constructor and operator from Process Consequently, applies rule-of-five. And springle some noexcepts in the special member functions. Add a catch(...) clause log path directory to always under LOCALAPPDATA Thus, renaming MakePathRelativeToEnvDir to UnderLocalAppDataPath Throws an exception if the LOCALAPPDATA path is too big to fit in a MAX_PATH sized buffer. Uses at instead of [] to index vectors The at method is checked and throw exceptions if out-of-bounds. Avoid overwrite previous logs operator= must return a value ;) Plus some pedantic header inclusion --- msix/ubuntu-pro-agent-launcher/console.cpp | 4 ++-- msix/ubuntu-pro-agent-launcher/console.hpp | 21 ++++++++++++++++++++- msix/ubuntu-pro-agent-launcher/error.cpp | 12 ++++++++---- msix/ubuntu-pro-agent-launcher/error.hpp | 5 ++--- msix/ubuntu-pro-agent-launcher/main.cpp | 13 ++++++++++--- 5 files changed, 42 insertions(+), 13 deletions(-) diff --git a/msix/ubuntu-pro-agent-launcher/console.cpp b/msix/ubuntu-pro-agent-launcher/console.cpp index a573a03e6..6c8287e2b 100644 --- a/msix/ubuntu-pro-agent-launcher/console.cpp +++ b/msix/ubuntu-pro-agent-launcher/console.cpp @@ -90,7 +90,7 @@ Process PseudoConsole::StartProcess(std::wstring commandLine) { si.StartupInfo.dwFlags = STARTF_USESTDHANDLES; si.lpAttributeList = attributes.get(); - Process p{0}; + Process p{}; if (!CreateProcessW(NULL, commandLine.data(), NULL, NULL, FALSE, EXTENDED_STARTUPINFO_PRESENT, NULL, NULL, &si.StartupInfo, &p)) { @@ -133,7 +133,7 @@ int EventLoop::Run() { DispatchMessage(&msg); } else { // invoke the listener subscribed to the handle that was signaled. - if (auto done = listeners_[signaledIndex](handles_[signaledIndex]); + if (auto done = listeners_.at(signaledIndex)(handles_.at(signaledIndex)); done.has_value()) { return done.value(); } diff --git a/msix/ubuntu-pro-agent-launcher/console.hpp b/msix/ubuntu-pro-agent-launcher/console.hpp index 8af931d17..36ee0f457 100644 --- a/msix/ubuntu-pro-agent-launcher/console.hpp +++ b/msix/ubuntu-pro-agent-launcher/console.hpp @@ -5,13 +5,32 @@ #include #include #include +#include #include +#include namespace up4w { // An RAII wrapper around the PROCESS_INFORMATION structure to ease preventing // HANDLE leaks. struct Process : PROCESS_INFORMATION { - ~Process() { + Process(Process const& other) = delete; + Process& operator=(Process const& other) = delete; + Process(Process&& other) noexcept { *this = std::move(other); } + Process& operator=(Process&& other) noexcept { + hProcess = std::exchange(other.hProcess, nullptr); + hThread = std::exchange(other.hThread, nullptr); + dwProcessId = std::exchange(other.dwProcessId, 0); + dwThreadId = std::exchange(other.dwThreadId, 0); + return *this; + } + Process() noexcept { + hProcess = nullptr; + hThread = nullptr; + dwProcessId = 0; + dwThreadId = 0; + } + + ~Process() noexcept { if (hThread != nullptr && hThread != INVALID_HANDLE_VALUE) { CloseHandle(hThread); } diff --git a/msix/ubuntu-pro-agent-launcher/error.cpp b/msix/ubuntu-pro-agent-launcher/error.cpp index c040ecc94..fcc312c60 100644 --- a/msix/ubuntu-pro-agent-launcher/error.cpp +++ b/msix/ubuntu-pro-agent-launcher/error.cpp @@ -3,8 +3,8 @@ #include namespace up4w { -std::wstring MakePathRelativeToEnvDir(std::wstring_view destination, - std::wstring_view envDir) { +std::wstring UnderLocalAppDataPath(std::wstring_view destination) { + std::wstring_view localAppData = L"LOCALAPPDATA"; std::wstring resultPath{}; resultPath.resize(MAX_PATH); @@ -12,11 +12,15 @@ std::wstring MakePathRelativeToEnvDir(std::wstring_view destination, static_cast(resultPath.capacity() - destination.size() - 1); auto length = - GetEnvironmentVariable(envDir.data(), resultPath.data(), truncatedLength); + GetEnvironmentVariable(localAppData.data(), resultPath.data(), truncatedLength); if (length == 0) { return {}; } + if (length > truncatedLength) { + throw hresult_exception{CO_E_PATHTOOLONG}; + } + resultPath.insert(length, destination.data()); return resultPath; } @@ -26,7 +30,7 @@ void LogSingleShot(std::filesystem::path const& logFilePath, auto const time = std::chrono::current_zone()->to_local(std::chrono::system_clock::now()); - std::ofstream logfile{logFilePath}; + std::ofstream logfile{logFilePath, std::ios::app}; logfile << std::format("{:%Y-%m-%d %T}: {}\n", time, message); } diff --git a/msix/ubuntu-pro-agent-launcher/error.hpp b/msix/ubuntu-pro-agent-launcher/error.hpp index 0693b3d0a..a818ec93f 100644 --- a/msix/ubuntu-pro-agent-launcher/error.hpp +++ b/msix/ubuntu-pro-agent-launcher/error.hpp @@ -54,10 +54,9 @@ class hresult_exception { }; /// Computes the absolute path resulting of joining the [destination] into the -/// value of the environment variable [envDir]. Returns empty string if the +/// value of the environment variable [LOCALAPPDATA]. Returns empty string if the /// environment variable is undefined. -std::wstring MakePathRelativeToEnvDir(std::wstring_view destination, - std::wstring_view envDir); +std::wstring UnderLocalAppDataPath(std::wstring_view destination); // Opens the log file, writes the message with a timestamp and closes it. void LogSingleShot(std::filesystem::path const& logFilePath, diff --git a/msix/ubuntu-pro-agent-launcher/main.cpp b/msix/ubuntu-pro-agent-launcher/main.cpp index 29974d345..884dd870c 100644 --- a/msix/ubuntu-pro-agent-launcher/main.cpp +++ b/msix/ubuntu-pro-agent-launcher/main.cpp @@ -12,9 +12,8 @@ #include "error.hpp" std::filesystem::path const& logPath() { - static std::filesystem::path localAppDataPath = - up4w::MakePathRelativeToEnvDir( - L"\\Ubuntu Pro\\ubuntu-pro-agent-launcher.log", L"LOCALAPPDATA"); + static std::filesystem::path localAppDataPath = up4w::UnderLocalAppDataPath( + L"\\Ubuntu Pro\\ubuntu-pro-agent-launcher.log"); return localAppDataPath; } @@ -80,4 +79,12 @@ int WINAPI wWinMain(HINSTANCE, HINSTANCE, PWSTR pCmdLine, int) try { up4w::LogSingleShot(localAppDataPath, err.what()); return 3; +} catch (...) { + std::filesystem::path const& localAppDataPath = logPath(); + if (localAppDataPath.empty()) { + return 1; + } + + up4w::LogSingleShot(localAppDataPath, "An unknown exception was thrown.\n"); + return 4; }