Isolate plugins in an out-of-process host (wslpluginhost.exe)#40769
Open
benhillis wants to merge 1 commit into
Open
Isolate plugins in an out-of-process host (wslpluginhost.exe)#40769benhillis wants to merge 1 commit into
benhillis wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements an alternative callback re-entrancy strategy for the out-of-process WSL plugin host design: instead of adding a second callback lock, it introduces a threaded notification + callback “pump” so plugin callbacks are executed back on the original notifying thread (preserving the existing single m_instanceLock re-entrancy model).
Changes:
- Add a new out-of-process COM local server
wslpluginhost.exeimplementingIWslPluginHostand forwarding plugin API callbacks back to the service viaIWslPluginHostCallback. - Refactor
PluginManagerto activate per-plugin hosts via COM (GIT marshaling), run outbound notifications throughPluginCallPump, and route inbound callbacks via the pump or a timed direct-lock path to avoid deadlocks. - Expand/adjust Windows tests and installer/packaging to include
wslpluginhost.exeand validate host-crash isolation + concurrency/callback scenarios.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/windows/testplugin/Plugin.cpp | Extends test plugin to exercise host-crash, concurrent callbacks, async worker callbacks, and teardown races. |
| test/windows/PluginTests.h | Adds new PluginTestType cases for out-of-proc host scenarios. |
| test/windows/PluginTests.cpp | Adds/updates tests to validate host crash behavior, concurrent callbacks, async worker callbacks, and teardown-race stability. |
| test/windows/InstallerTests.cpp | Ensures installed executable set includes wslpluginhost.exe. |
| test/windows/Common.h | Adds wslpluginhost.exe to test “BinaryUnderTest” list; declares service PID helpers. |
| test/windows/Common.cpp | Implements GetWslServicePid() and GetWslServiceRunningPid() to validate service survival without silent restarts. |
| src/windows/wslpluginhost/exe/resource.h | Adds resource declarations for the new plugin host executable. |
| src/windows/wslpluginhost/exe/PluginHost.h | Declares the COM host class that loads plugin DLLs and forwards callbacks to the service. |
| src/windows/wslpluginhost/exe/PluginHost.cpp | Implements plugin host initialization, hook dispatch, and service-callback stubs (including COM init on worker threads). |
| src/windows/wslpluginhost/exe/main.rc | Adds version/icon resources for wslpluginhost.exe. |
| src/windows/wslpluginhost/exe/main.cpp | Implements COM local server lifecycle (factory registration, mitigation policies, startup timeout). |
| src/windows/wslpluginhost/exe/CMakeLists.txt | Builds the new wslpluginhost target and links required libraries/headers. |
| src/windows/wslpluginhost/CMakeLists.txt | Adds plugin host subdirectory to the build. |
| src/windows/wslinstall/DllMain.cpp | Registers wslpluginhost.exe in LSP categories list. |
| src/windows/service/stub/CMakeLists.txt | Adds generated proxy/stub sources for WslPluginHost.idl into wslserviceproxystub.dll. |
| src/windows/service/inc/WslPluginHost.idl | Introduces COM interfaces IWslPluginHost and IWslPluginHostCallback plus CLSID. |
| src/windows/service/inc/CMakeLists.txt | Adds wslpluginhostidl generation. |
| src/windows/service/exe/WSLCSessionManager.h | Reworks session iteration helpers to avoid holding session lock across out-of-proc plugin notifications. |
| src/windows/service/exe/WSLCSessionManager.cpp | Moves stopping notifications out from under session lock; adds rollback/race handling around OnWslcSessionCreated. |
| src/windows/service/exe/ServiceMain.cpp | Calls g_pluginManager.Shutdown() before CoUninitialize to avoid proxy teardown after COM shutdown. |
| src/windows/service/exe/PluginManager.h | Major refactor for out-of-proc hosts, GIT proxying, job object ownership, WSLC session ref-map, and pump routing. |
| src/windows/service/exe/PluginManager.cpp | Implements out-of-proc activation, host-crash latching, pump-driven notifications, and callback routing logic. |
| src/windows/service/exe/PluginCallPump.h | Adds the pump primitive for running notifications on a worker while executing callbacks on the notifying thread. |
| src/windows/service/exe/PluginCallPump.cpp | Implements pump queueing, worker coordination, and safe shutdown semantics. |
| src/windows/service/exe/LxssUserSession.h | Adds TryInvokeUnderInstanceLock to support timed direct-callback execution without deadlocking. |
| src/windows/service/exe/LxssUserSession.cpp | Implements TryInvokeUnderInstanceLock. |
| src/windows/service/exe/CMakeLists.txt | Adds PluginCallPump and wslpluginhostidl dependency to wslservice. |
| src/windows/common/precomp.h | Adds <shared_mutex> include to shared PCH. |
| msipackage/package.wix.in | Installs wslpluginhost.exe and registers its COM/AppID/Interface entries. |
| msipackage/CMakeLists.txt | Adds wslpluginhost.exe to MSI packaging inputs and dependencies. |
| CMakeLists.txt | Adds src/windows/wslpluginhost to the top-level build. |
| .pipelines/build-stage.yml | Adds wslpluginhost to CI build targets/artifact patterns. |
deedac2 to
67d56d6
Compare
67d56d6 to
22ab719
Compare
22ab719 to
cdb7b18
Compare
benhillis
pushed a commit
that referenced
this pull request
Jun 18, 2026
WSL plugin DLLs are moved out of wslservice.exe into a separate wslpluginhost.exe COM server so plugin code can no longer crash or destabilize the service. Each plugin is activated in its own host process (CLSCTX_LOCAL_SERVER, SYSTEM-only via AppID) and reached through a versioned COM interface defined in WslPluginHost.idl. Adds the ThreadedPluginAPI (PluginCallPump) alternative and broadened plugin tests (parallel/async callbacks). The plugin API is unchanged; existing plugins run unmodified. Rebased onto origin/master (28447f5): squashed PR #40769 onto current master, reconciling the WSLCCompat IDL additions (#40784) alongside the new WslPluginHost IDL in the service inc/stub CMake targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ff5334e to
e812ff4
Compare
WSL plugin DLLs are moved out of wslservice.exe into a separate wslpluginhost.exe COM server so plugin code can no longer crash or destabilize the service. Each plugin is activated in its own host process (CLSCTX_LOCAL_SERVER, SYSTEM-only via AppID) and reached through a versioned COM interface defined in WslPluginHost.idl. Adds the ThreadedPluginAPI (PluginCallPump) alternative and broadened plugin tests (parallel/async callbacks). The plugin API is unchanged; existing plugins run unmodified. Rebased onto origin/master (28447f5): squashed PR #40769 onto current master, reconciling the WSLCCompat IDL additions (#40784) alongside the new WslPluginHost IDL in the service inc/stub CMake targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e812ff4 to
73c5ba5
Compare
Comment on lines
936
to
937
| g_logfile << "WSLC Session stopping, name=" << wsl::shared::string::WideToMultiByte(Session->DisplayName) | ||
| << ", id=" << Session->SessionId << std::endl; |
| signatureHandle = wsl::windows::common::install::ValidateFileSignature(PluginPath); | ||
| } | ||
|
|
||
| m_module.reset(LoadLibrary(PluginPath)); |
Comment on lines
+694
to
+701
| if (g_testType == PluginTestType::ParallelWslcWithCallbacks) | ||
| { | ||
| // Spawn a worker that waits for both sessions to arrive (barrier), then | ||
| // makes a callback. Concurrency tracking mirrors ConcurrentApiCalls but | ||
| // across two separate notification calls instead of threads from one hook. | ||
| const auto sessionId = Session->SessionId; | ||
| std::lock_guard<std::mutex> lock(g_parallelWorkersLock); | ||
| g_parallelWorkers.emplace_back([sessionId]() { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WSL plugin DLLs are moved out of
wslservice.exeinto a separatewslpluginhost.exeCOM server so plugin code can no longer crash or destabilize the service. Each plugin is activated in its own host process (CLSCTX_LOCAL_SERVER, SYSTEM-only via AppID) and reached through a versioned COM interface defined inWslPluginHost.idl. All hosts are tied to a service-owned job object and terminate whenwslserviceexits.The plugin API is unchanged and existing plugins run unmodified. Proxy/stub is consolidated into
wslserviceproxystub.dll-- one new exe, no new DLLs.Callback re-entrancy: threaded callback pump
While the service is blocked in an outbound notification (
host->On...), the plugin may call back into the service. In-process, those callbacks re-entered the session's recursivem_instanceLockon the same thread. Out-of-process they arrive on a different COM thread, so the model has to be reconstructed.This PR reproduces the in-process re-entrancy model with a single lock by running each outbound notification on a worker thread and pumping the plugin's service-side API calls back onto the original notifying thread, which already holds the session's recursive
m_instanceLock:PluginCallPump(src/windows/service/exe/PluginCallPump.{h,cpp}) --Run(notification)spawns a worker thread that makes the outbound COM call; the notifying thread pumps queuedInvoke(work)calls and runs them underm_instanceLock, so recursive locks re-enter exactly as in-process.Invokereports ran-vs-stopped out-of-band (no HRESULT sentinel), is exception-safe, and cannot strand an RPC thread.PluginManager.cpp) -- WSL notifications (OnVm*,OnDistribution*) route viaRunHostNotification, which registers a per-session pump for the duration of the call. WSL callbacks (MountFolder,ExecuteBinary[InDistribution]) route viaInvokeOnWslPump: pump when a hook is in flight, direct (ownm_instanceLock) otherwise -- preserving async out-of-hook callbacks.m_instanceLock.InvokeOnWslPumpuses a timed acquire (TryInvokeUnderInstanceLock) and re-checks for a pump, routing the racing callback onto the pump instead of dead-blocking.This keeps the single-lock model the rest of the session already assumes -- no second
m_callbackLock, no new dual-lock annotations spread across callback-reachable state.Host-crash classification
A crashing or disconnected host is classified by
IsHostCrashand surfaced as "host died, log and continue" rather than a fatal plugin error:RPC_E_DISCONNECTED,RPC_E_SERVER_DIED,RPC_E_SERVER_DIED_DNE,CO_E_OBJNOTCONNECTED,RPC_S_SERVER_UNAVAILABLE,RPC_S_CALL_FAILED,RPC_S_CALL_FAILED_DNE.RPC_E_CALL_REJECTEDis intentionally not classified as a host crash: it is a transient COM busy state (an STA message filter rejecting a call), not a "server process died" signal. The plugin host is MTA with no message filter, so it should not occur here; treating it as a crash would silently skip future legitimate calls.Start hooks abort on host death; teardown hooks latch-and-continue. The host assigns itself to the service-owned job object before loading any plugin code, so a service crash still tears down every host.
Scope note
The WSLC session-ref registry is intentionally left as-is (its lock is non-recursive, notifications already fire outside it, and it is entangled with the create-veto protocol). Documented as a follow-up.
New tests
wslpluginhost.exemid-OnVmStartedand verifies the service survives andm_initOncestays sticky.MountFolder+ExecuteBinary._VmTerminate's teardown, exiting deterministically via anOnVmStopping-set stop signal.Existing WSL1 plugin tests were broadened alongside the refactor.
Validation
Result:
Total=31, Passed=30, Failed=0, Skipped=1(skip = signed-build-only signature test). Includes the previously-deadlockingCallbacksDuringTerminationDoNotCrash. The branch is kept current withmasterby merge; full PR CI (x64/arm64 builds, package, wslc/wsl1/wsl2 test stages, formatting, CodeQL) runs on each push.Supersedes #40120 (same out-of-process host; that PR used a dual
m_callbackLock+ session-ref registry, replaced here by the single-lock threaded pump per review feedback).