From b9ca35e3734be4cf14a19bf7863f9a651d4493ff Mon Sep 17 00:00:00 2001 From: Blue Date: Tue, 9 Dec 2025 18:22:16 -0800 Subject: [PATCH] Apply suggestion --- src/windows/common/WSLAProcessLauncher.h | 6 ++--- .../exe/ServiceProcessLauncher.cpp | 22 ++++++++++++++++--- .../wslaservice/exe/ServiceProcessLauncher.h | 1 + src/windows/wslaservice/exe/WSLAContainer.cpp | 11 +++++++--- test/windows/WSLATests.cpp | 11 +++++----- 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/windows/common/WSLAProcessLauncher.h b/src/windows/common/WSLAProcessLauncher.h index 099306a..a772b6a 100644 --- a/src/windows/common/WSLAProcessLauncher.h +++ b/src/windows/common/WSLAProcessLauncher.h @@ -93,12 +93,12 @@ public: std::tuple> LaunchNoThrow(IWSLASession& Session); std::tuple> LaunchNoThrow(IWSLAContainer& Container); - std::string FormatResult(const RunningWSLAProcess::ProcessResult& IWSLAContainer); + std::string FormatResult(const RunningWSLAProcess::ProcessResult& Result); template - ClientRunningWSLAProcess Launch(T& Session) + auto Launch(T& Context) { - auto [hresult, error, process] = LaunchNoThrow(Session); + auto [hresult, error, process] = LaunchNoThrow(Context); if (FAILED(hresult)) { auto commandLine = wsl::shared::string::Join(m_arguments, ' '); diff --git a/src/windows/wslaservice/exe/ServiceProcessLauncher.cpp b/src/windows/wslaservice/exe/ServiceProcessLauncher.cpp index 0991fdf..7ccef9f 100644 --- a/src/windows/wslaservice/exe/ServiceProcessLauncher.cpp +++ b/src/windows/wslaservice/exe/ServiceProcessLauncher.cpp @@ -51,10 +51,26 @@ void ServiceRunningProcess::GetState(WSLA_PROCESS_STATE* State, int* Code) THROW_IF_FAILED(m_process->GetState(State, Code)); } -ServiceRunningProcess ServiceProcessLauncher::Launch(WSLAVirtualMachine& virtualMachine) +std::tuple> ServiceProcessLauncher::LaunchNoThrow(WSLAVirtualMachine& virtualMachine) { auto [options, commandLine, env] = CreateProcessOptions(); - int Error = -1; + int error = -1; - return ServiceRunningProcess(virtualMachine.CreateLinuxProcess(options, &Error), std::move(m_fds)); + std::optional process; + auto result = + wil::ResultFromException([&]() { process.emplace(virtualMachine.CreateLinuxProcess(options, &error), std::move(m_fds)); }); + + return {result, error, std::move(process)}; } + +ServiceRunningProcess ServiceProcessLauncher::Launch(WSLAVirtualMachine& virtualMachine) +{ + auto [hresult, error, process] = LaunchNoThrow(virtualMachine); + if (FAILED(hresult)) + { + auto commandLine = wsl::shared::string::Join(m_arguments, ' '); + THROW_HR_MSG(hresult, "Failed to launch process: %hs (commandline: %hs). Errno = %i", m_executable.c_str(), commandLine.c_str(), error); + } + + return std::move(process.value()); +} \ No newline at end of file diff --git a/src/windows/wslaservice/exe/ServiceProcessLauncher.h b/src/windows/wslaservice/exe/ServiceProcessLauncher.h index 88b4b06..5c94299 100644 --- a/src/windows/wslaservice/exe/ServiceProcessLauncher.h +++ b/src/windows/wslaservice/exe/ServiceProcessLauncher.h @@ -45,6 +45,7 @@ public: NON_MOVABLE(ServiceProcessLauncher); using WSLAProcessLauncher::WSLAProcessLauncher; + std::tuple> LaunchNoThrow(WSLAVirtualMachine& virtualMachine); ServiceRunningProcess Launch(WSLAVirtualMachine& virtualMachine); }; } // namespace wsl::windows::service::wsla \ No newline at end of file diff --git a/src/windows/wslaservice/exe/WSLAContainer.cpp b/src/windows/wslaservice/exe/WSLAContainer.cpp index 1d4cdd6..8a945b3 100644 --- a/src/windows/wslaservice/exe/WSLAContainer.cpp +++ b/src/windows/wslaservice/exe/WSLAContainer.cpp @@ -162,6 +162,7 @@ CATCH_RETURN(); HRESULT WSLAContainer::Exec(const WSLA_PROCESS_OPTIONS* Options, IWSLAProcess** Process, int* Errno) try { + *Errno = -1; THROW_HR_IF_MSG(E_INVALIDARG, Options->Executable != nullptr, "Executable must be null"); std::lock_guard lock{m_lock}; @@ -202,8 +203,12 @@ try launcher.AddFd(Options->Fds[i]); } - auto process = launcher.Launch(*m_parentVM); - THROW_IF_FAILED(process.Get().QueryInterface(__uuidof(IWSLAProcess), (void**)Process)); + std::optional process; + HRESULT result = E_FAIL; + std::tie(result, *Errno, process) = launcher.LaunchNoThrow(*m_parentVM); + THROW_IF_FAILED(result); + + THROW_IF_FAILED(process->Get().QueryInterface(__uuidof(IWSLAProcess), (void**)Process)); return S_OK; } @@ -233,7 +238,7 @@ void WSLAContainer::AddEnvironmentVariables(std::vector& args, cons { for (ULONG i = 0; i < options.EnvironmentCount; i++) { - THROW_HR_IF_MSG(E_INVALIDARG, options.Environment[i][0] == L'-', "Invalid environment string: %hs", options.Environment[i]); + THROW_HR_IF_MSG(E_INVALIDARG, options.Environment[i][0] == '-', "Invalid environment string: %hs", options.Environment[i]); args.insert(args.end(), {"-e", options.Environment[i]}); } diff --git a/test/windows/WSLATests.cpp b/test/windows/WSLATests.cpp index 47c30ed..8ef8b96 100644 --- a/test/windows/WSLATests.cpp +++ b/test/windows/WSLATests.cpp @@ -198,6 +198,7 @@ class WSLATests WSL2_TEST_ONLY(); auto settings = GetDefaultSessionSettings(); + settings.StoragePath = nullptr; settings.DisplayName = L"wsla-test-list"; wil::com_ptr userSession; @@ -210,11 +211,10 @@ class WSLATests // Act: list sessions wil::unique_cotaskmem_array_ptr sessions; - ULONG count = 0; - VERIFY_SUCCEEDED(userSession->ListSessions(&sessions, &count)); + VERIFY_SUCCEEDED(userSession->ListSessions(&sessions, sessions.size_address())); // Assert - VERIFY_ARE_EQUAL(count, 1u); + VERIFY_ARE_EQUAL(sessions.size(), 1); const auto& info = sessions[0]; // SessionId is implementation detail (starts at 1), so we only assert DisplayName here. @@ -226,6 +226,7 @@ class WSLATests WSL2_TEST_ONLY(); auto settings = GetDefaultSessionSettings(); + settings.StoragePath = nullptr; settings.DisplayName = L"wsla-open-by-name-test"; wil::com_ptr userSession; @@ -1367,7 +1368,7 @@ class WSLATests } // Validate that stdin is correctly wired. - // TODO: Add test coverage for stdin being closed without anything written written to it once the stdin hang issue is solved. + // TODO: Add test coverage for stdin being closed without anything written to it once the stdin hang issue is solved. { auto process = WSLAProcessLauncher({}, {"/bin/cat"}, {}, ProcessFlags::Stdin | ProcessFlags::Stdout | ProcessFlags::Stderr) .Launch(container.Get()); @@ -1402,7 +1403,7 @@ class WSLATests .Launch(container.Get()); // TODO: Replace with Stop() - ExpectCommandResult(session.get(), {"/usr/bin/nerdctl", "stop", "test-container-exec"}, 0); + ExpectCommandResult(session.get(), {"/usr/bin/nerdctl", "stop", "-t", "0", "test-container-exec"}, 0); auto result = process.WaitAndCaptureOutput(); VERIFY_ARE_EQUAL(result.Code, 1);