-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement SDK IO callbacks #14462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/wsl-for-apps
Are you sure you want to change the base?
Implement SDK IO callbacks #14462
Changes from all commits
4ac0567
5dc7be0
2f165d1
22ca217
e1cc982
750998d
488ffb7
fbb244b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /*++ | ||
| Copyright (c) Microsoft. All rights reserved. | ||
| Module Name: | ||
| IOCallback.cpp | ||
| Abstract: | ||
| Holds IO callback objects. | ||
| --*/ | ||
| #include "precomp.h" | ||
| #include "WslcsdkPrivate.h" | ||
|
|
||
| IOCallback::IOCallback(IWSLAProcess* process, const WslcContainerProcessIOCallbackOptions& options) | ||
| { | ||
| using namespace wsl::windows::common::relay; | ||
|
|
||
| auto addIOCallback = [&](WslcProcessIOHandle ioHandle, WslcStdIOCallback callback, PVOID context) { | ||
| std::function<void(const gsl::span<char>& Buffer)> function; | ||
| if (callback) | ||
| { | ||
| function = [callback, context](const gsl::span<char>& buffer) { | ||
| callback(reinterpret_cast<const BYTE*>(buffer.data()), static_cast<uint32_t>(buffer.size()), context); | ||
| }; | ||
| } | ||
| else | ||
| { | ||
| function = [](const gsl::span<char>&) {}; | ||
| } | ||
|
|
||
| m_io.AddHandle(std::make_unique<ReadHandle>(GetIOHandle(process, ioHandle), std::move(function))); | ||
| }; | ||
|
|
||
| addIOCallback(WSLC_PROCESS_IO_HANDLE_STDOUT, options.stdOutCallback, options.stdOutCallbackContext); | ||
| addIOCallback(WSLC_PROCESS_IO_HANDLE_STDERR, options.stdErrCallback, options.stdErrCallbackContext); | ||
|
|
||
| m_io.AddHandle(std::make_unique<EventHandle>(m_cancelEvent.get()), MultiHandleWait::CancelOnCompleted); | ||
|
|
||
| m_thread = std::thread([this]() { | ||
| try | ||
| { | ||
| m_io.Run({}); | ||
| } | ||
| CATCH_LOG(); | ||
| }); | ||
| } | ||
|
|
||
| IOCallback::~IOCallback() | ||
| { | ||
| Cancel(); | ||
| if (m_thread.joinable()) | ||
| { | ||
| m_thread.join(); | ||
| } | ||
JohnMcPMS marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| void IOCallback::Cancel() | ||
| { | ||
| m_cancelEvent.SetEvent(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Looks like this is never called separately, so might as well have everything in the destructor |
||
| } | ||
|
|
||
| bool IOCallback::HasIOCallback(const WslcContainerProcessOptionsInternal* options) | ||
| { | ||
| return options && HasIOCallback(options->ioCallbacks); | ||
| } | ||
|
|
||
| bool IOCallback::HasIOCallback(const WslcContainerProcessIOCallbackOptions& options) | ||
| { | ||
| return options.stdOutCallback || options.stdErrCallback; | ||
| } | ||
|
|
||
| wil::unique_handle IOCallback::GetIOHandle(IWSLAProcess* process, WslcProcessIOHandle ioHandle) | ||
| { | ||
| ULONG ulongHandle = 0; | ||
|
|
||
| THROW_IF_FAILED(process->GetStdHandle(static_cast<ULONG>(static_cast<std::underlying_type_t<WslcProcessIOHandle>>(ioHandle)), &ulongHandle)); | ||
|
|
||
| return wil::unique_handle{ULongToHandle(ulongHandle)}; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| /*++ | ||
|
|
||
| Copyright (c) Microsoft. All rights reserved. | ||
|
|
||
| Module Name: | ||
|
|
||
| IOCallback.h | ||
|
|
||
| Abstract: | ||
|
|
||
| Holds IO callback objects. | ||
|
|
||
| --*/ | ||
| #pragma once | ||
| #include "wslaservice.h" | ||
| #include "relay.hpp" | ||
| #include <thread> | ||
|
|
||
| struct WslcContainerProcessIOCallbackOptions; | ||
| struct WslcContainerProcessOptionsInternal; | ||
|
|
||
| struct IOCallback | ||
| { | ||
| IOCallback(IWSLAProcess* process, const WslcContainerProcessIOCallbackOptions& options); | ||
| ~IOCallback(); | ||
|
|
||
| void Cancel(); | ||
|
|
||
| static bool HasIOCallback(const WslcContainerProcessOptionsInternal* options); | ||
| static bool HasIOCallback(const WslcContainerProcessIOCallbackOptions& options); | ||
|
|
||
| static wil::unique_handle GetIOHandle(IWSLAProcess* process, WslcProcessIOHandle ioHandle); | ||
|
|
||
| private: | ||
| std::thread m_thread; | ||
| wsl::windows::common::relay::MultiHandleWait m_io; | ||
| wil::unique_event m_cancelEvent{wil::EventOptions::ManualReset}; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -535,6 +535,12 @@ try | |
| if (SUCCEEDED(errorInfoWrapper.CaptureResult(internalSession->session->CreateContainer(&containerOptions, &result->container)))) | ||
| { | ||
| wsl::windows::common::security::ConfigureForCOMImpersonation(result->container.get()); | ||
|
|
||
| if (IOCallback::HasIOCallback(internalContainerSettings->initProcessOptions)) | ||
| { | ||
| result->ioCallbackOptions = internalContainerSettings->initProcessOptions->ioCallbacks; | ||
| } | ||
|
|
||
| *container = reinterpret_cast<WslcContainer>(result.release()); | ||
| } | ||
|
|
||
|
|
@@ -549,7 +555,23 @@ try | |
| auto internalType = CheckAndGetInternalType(container); | ||
| RETURN_HR_IF_NULL(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), internalType->container); | ||
|
|
||
| return errorInfoWrapper.CaptureResult(internalType->container->Start(ConvertFlags(flags), nullptr)); | ||
| bool hasIOCallback = IOCallback::HasIOCallback(internalType->ioCallbackOptions); | ||
| // If callbacks were provided, ATTACH must be used. | ||
| // TODO: Consider if we should just override flags when callbacks were provided instead. | ||
| RETURN_HR_IF(E_INVALIDARG, WI_IsFlagClear(flags, WSLC_CONTAINER_START_FLAG_ATTACH) && hasIOCallback); | ||
|
|
||
| if (SUCCEEDED(errorInfoWrapper.CaptureResult(internalType->container->Start(ConvertFlags(flags), nullptr)))) | ||
| { | ||
| if (hasIOCallback) | ||
| { | ||
| wil::com_ptr<IWSLAProcess> process; | ||
| RETURN_IF_FAILED(internalType->container->GetInitProcess(&process)); | ||
| wsl::windows::common::security::ConfigureForCOMImpersonation(process.get()); | ||
| internalType->ioCallbacks = std::make_shared<IOCallback>(process.get(), internalType->ioCallbackOptions); | ||
| } | ||
| } | ||
|
|
||
| return errorInfoWrapper; | ||
| } | ||
| CATCH_RETURN(); | ||
|
|
||
|
|
@@ -680,6 +702,12 @@ try | |
| if (SUCCEEDED(errorInfoWrapper.CaptureResult(internalContainer->container->Exec(&runtimeOptions, nullptr, &result->process)))) | ||
| { | ||
| wsl::windows::common::security::ConfigureForCOMImpersonation(result->process.get()); | ||
|
|
||
| if (IOCallback::HasIOCallback(internalProcessSettings)) | ||
| { | ||
| result->ioCallbacks = std::make_shared<IOCallback>(result->process.get(), internalProcessSettings->ioCallbacks); | ||
| } | ||
|
|
||
| *newProcess = reinterpret_cast<WslcProcess>(result.release()); | ||
| } | ||
|
|
||
|
|
@@ -724,6 +752,9 @@ try | |
| RETURN_IF_FAILED(internalType->container->GetInitProcess(&result->process)); | ||
|
|
||
| wsl::windows::common::security::ConfigureForCOMImpersonation(result->process.get()); | ||
|
|
||
| result->ioCallbacks = internalType->ioCallbacks.load(); | ||
|
|
||
| *initProcess = reinterpret_cast<WslcProcess>(result.release()); | ||
|
|
||
| return S_OK; | ||
|
|
@@ -914,19 +945,30 @@ try | |
| } | ||
| CATCH_RETURN(); | ||
|
|
||
| STDAPI WslcSetProcessSettingsIoCallback( | ||
| _In_ WslcProcessSettings* processSettings, _In_ WslcProcessIoHandle ioHandle, _In_opt_ WslcStdIOCallback stdIOCallback, _In_opt_ PVOID context) | ||
| STDAPI WslcSetProcessSettingsIOCallback( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just realizing this now, but we need a way to unregister the IO callbacks and synchronize them. Something like: (Once that method returns, the caller is guaranteed that all pending callbacks are completed and that no future callback can be received).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current design is that releasing all of the process/container objects related to the process does that. Container+all retrieved InitProcess instances for init processes, just the one process for any created afterward. We could additionally have a way to explicitly do it, but I don't think that it is strictly necessary. It would have to take in a A reason that this might be needed is to ensure that ALL output has been received (the buffers flushed) before teardown as the current release design cancels any future data flow. If the process is still running that isn't really an issue. If it has stopped, we know that there is an end to the pipe and we should get there in reasonably short order. I think the caller flow would be like: the implementation for (pseudoname)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately this won't be sufficient because the service doesn't guarantee that all the pending IO has been written the pipe after the exit event is signaled. For context, two things happen when a process terminates
But those can happen in either order. Thinking through this, I think this might be something that we've overlooked when we designed this. The only way I can think of fixing this in a way that's not potentially confusing for the other would be something like: We could make this work by having
And only call Forcing the user to register all callbacks at once will push them to implement this correctly and only release the process after OnExited is called. |
||
| _In_ WslcProcessSettings* processSettings, _In_ WslcProcessIOHandle ioHandle, _In_opt_ WslcStdIOCallback stdIOCallback, _In_opt_ PVOID context) | ||
| try | ||
| { | ||
| UNREFERENCED_PARAMETER(processSettings); | ||
| UNREFERENCED_PARAMETER(ioHandle); | ||
| UNREFERENCED_PARAMETER(stdIOCallback); | ||
| UNREFERENCED_PARAMETER(context); | ||
| return E_NOTIMPL; | ||
| auto internalType = CheckAndGetInternalType(processSettings); | ||
| RETURN_HR_IF(E_INVALIDARG, ioHandle != WSLC_PROCESS_IO_HANDLE_STDOUT && ioHandle != WSLC_PROCESS_IO_HANDLE_STDERR); | ||
| RETURN_HR_IF(E_INVALIDARG, stdIOCallback == nullptr && context != nullptr); | ||
|
|
||
| if (ioHandle == WSLC_PROCESS_IO_HANDLE_STDOUT) | ||
| { | ||
| internalType->ioCallbacks.stdOutCallback = stdIOCallback; | ||
| internalType->ioCallbacks.stdOutCallbackContext = context; | ||
| } | ||
| else if (ioHandle == WSLC_PROCESS_IO_HANDLE_STDERR) | ||
| { | ||
| internalType->ioCallbacks.stdErrCallback = stdIOCallback; | ||
| internalType->ioCallbacks.stdErrCallbackContext = context; | ||
| } | ||
|
|
||
| return S_OK; | ||
| } | ||
| CATCH_RETURN(); | ||
|
|
||
| STDAPI WslcGetProcessIOHandle(_In_ WslcProcess process, _In_ WslcProcessIoHandle ioHandle, _Out_ HANDLE* handle) | ||
| STDAPI WslcGetProcessIOHandle(_In_ WslcProcess process, _In_ WslcProcessIOHandle ioHandle, _Out_ HANDLE* handle) | ||
| try | ||
| { | ||
| auto internalType = CheckAndGetInternalType(process); | ||
|
|
@@ -935,17 +977,10 @@ try | |
|
|
||
| *handle = nullptr; | ||
|
|
||
| ULONG ulongHandle = 0; | ||
|
|
||
| HRESULT hr = internalType->process->GetStdHandle( | ||
| static_cast<ULONG>(static_cast<std::underlying_type_t<WslcProcessIoHandle>>(ioHandle)), &ulongHandle); | ||
| auto result = IOCallback::GetIOHandle(internalType->process.get(), ioHandle); | ||
| *handle = result.release(); | ||
|
|
||
| if (SUCCEEDED_LOG(hr)) | ||
| { | ||
| *handle = ULongToHandle(ulongHandle); | ||
| } | ||
|
|
||
| return hr; | ||
| return S_OK; | ||
| } | ||
| CATCH_RETURN(); | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.