Skip to content

Wslc events#40971

Open
kvega005 wants to merge 28 commits into
microsoft:masterfrom
kvega005:wslcEvents
Open

Wslc events#40971
kvega005 wants to merge 28 commits into
microsoft:masterfrom
kvega005:wslcEvents

Conversation

@kvega005

@kvega005 kvega005 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Adds a docker events style event stream to the WSLC session. A new IWSLCEventStream COM interface and IWSLCSession::GetEvents let callers subscribe to container lifecycle events (create, start, kill, stop, destroy), filtered by time window and key/value filters, pulled one JSON event at a time. Events are backed by a new bounded in-memory ring (EventStore) fed from Docker's own event stream.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings July 1, 2026 22:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Docker-like event streaming capability to the WSLC session so clients can subscribe to container lifecycle events over a new COM stream interface, backed by a bounded in-memory event ring shared across subscribers.

Changes:

  • Introduces IWSLCEventStream and IWSLCSession::GetEvents for pull-based, filtered event consumption over a time window.
  • Implements an EventStore ring buffer and wires event recording from Docker container lifecycle notifications.
  • Extends SDK/IDL/package/localization assets and adds new end-to-end tests for event streaming behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/windows/WSLCTests.cpp Adds tests for event stream ordering/filtering and termination-unblocks-reader behavior.
src/windows/wslcsession/WSLCSession.h Declares GetEvents and adds EventStore as a session member.
src/windows/wslcsession/WSLCSession.cpp Implements WSLCSession::GetEvents and triggers event-store termination wake on session terminate.
src/windows/wslcsession/WSLCContainer.h Plumbs EventStore into container implementation and clarifies timestamp units.
src/windows/wslcsession/WSLCContainer.cpp Records create/start/kill/stop/destroy lifecycle events into EventStore.
src/windows/wslcsession/EventStore.h Adds EventStore + EventStream COM runtime class definitions.
src/windows/wslcsession/EventStore.cpp Implements bounded ring buffering, filtering, and blocking GetNext behavior.
src/windows/wslcsession/DockerEventTracker.h Adds Kill to tracked container events.
src/windows/wslcsession/DockerEventTracker.cpp Maps Docker "kill" action to the new ContainerEvent::Kill.
src/windows/wslcsession/CMakeLists.txt Adds EventStore sources/headers to the build.
src/windows/WslcSDK/wslcsdk.h Adds new HRESULTs for events-lost and stream-finished conditions.
src/windows/service/inc/wslc.idl Adds IWSLCEventStream and IWSLCSession::GetEvents to the COM contract.
src/windows/inc/wslc_schema.h Adds JSON schema structs for event serialization/deserialization.
msipackage/package.wix.in Registers the new COM interface IID for proxy/stub.
localization/strings/en-US/Resources.resw Adds a localized user-facing message for invalid event time windows.
Comments suppressed due to low confidence (3)

test/windows/WSLCTests.cpp:5859

  • until is captured before RunningWSLCContainer goes out of scope; its destructor calls Delete(...) which is what triggers Docker's destroy event. That can place the destroy event outside the [since, until] window, making this test flaky (and causing events[4].time <= until to fail intermittently). Capture until after the container is deleted (after the scope), so the window includes the destroy timestamp.
            launcher.AddTmpfs("relative-path", "");

            auto [hresult, container] = launcher.LaunchNoThrow(*m_defaultSession);
            VERIFY_ARE_EQUAL(hresult, E_FAIL);

test/windows/WSLCTests.cpp:5930

  • This test can hang indefinitely on failure: if GetNext doesn't unblock, future.get() will block, and the unconditional readerThread.join() in the scope-exit will also block (including during stack unwinding after a VERIFY failure). Prefer joining only after confirming readiness, and detach + fail when the timeout elapses so the test fails fast instead of wedging the suite.
            VERIFY_SUCCEEDED(container->Delete(WSLCDeleteFlagsNone));
        }

        // Validate that invalid tty sizes are rejected.
        {

src/windows/service/inc/wslc.idl:554

  • The comment says the returned JSON matches Docker's events --format json shape including timeNano, but the current wslc_schema::Event only defines Type, Action, Actor, and time (no timeNano). The comment also doesn't document the completion/loss HRESULTs that callers must handle. Update the IDL comment to match the actual schema and to mention WSLC_E_EVENT_STREAM_FINISHED/WSLC_E_EVENTS_LOST (and E_ABORT on termination).
    [unique, size_is(ContainersCount)] WSLCContainerId* Containers;
    ULONG ContainersCount;
    ULONGLONG SpaceReclaimed;
} WSLCPruneContainersResults;

Comment thread src/windows/wslcsession/EventStore.cpp
Comment thread src/windows/wslcsession/EventStore.cpp Outdated
Copilot AI review requested due to automatic review settings July 1, 2026 22:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Comment thread test/windows/WSLCTests.cpp
Comment thread localization/strings/en-US/Resources.resw
Comment thread src/windows/service/inc/wslc.idl
Copilot AI review requested due to automatic review settings July 1, 2026 23:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comment thread src/windows/wslcsession/EventStore.cpp
@kvega005 kvega005 marked this pull request as ready for review July 2, 2026 18:44
@kvega005 kvega005 requested a review from a team as a code owner July 2, 2026 18:44
Copilot AI review requested due to automatic review settings July 2, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

m_updated.notify_all();
}

void EventStore::Record(std::string Type, std::string Action, const std::string& ActorId, std::optional<uint64_t> TimeSeconds) noexcept

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We could probably make the strings rvalue references to avoid copies here since the caller will most likely be able to move them all

return m_events[index];
}

bool EventStore::WaitForEvent(std::unique_lock<std::mutex>& Lock, uint64_t SequenceNumber, std::optional<uint64_t> Until)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary for this change, but ideally we should also wait for the caller's process, otherwise if a user runs wslc events and then ctrl-c, a thread on the service side could get stuck here until an event is emitted to unblock it

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly condition_variables don't easily let us wait for multiple events so we might need to make our own for this

if (event == ContainerEvent::Stop)
// Record lifecycle events here, driven by Docker's own events, so the store gets Docker's
// timestamps and the same ordering Docker observed.
if (event == ContainerEvent::Start)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I think we should report these events after we've updated our internal state, otherwise a caller could see the container stop event and let's say try to call container.Start() again but if we haven't internally transitioned to the stopped state, their call would race with ours.

My intuition there would be to emit those events in the Transition() method, but we're currently missing the event time when we start the container (should be easy to fix though)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I think we're going to need to redesign the container state machine a bit (mostly because I recently realized that if the caller makes a stop call with an infinite timeout, any Kill() call will get stuck)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants