Wslc events

[Kevin Vega (kvega005)]

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]

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;

[Copilot]

Pull request overview

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

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/windows/wslcsession/EventStore.cpp:35

• Append() claims Docker delivery order is also timestamp order and enforces it with WI_ASSERT(m_events.back().time <= Event.time), but later in Get() the comment explicitly says Docker timestamps are not an ordering key. These statements conflict; if timestamps can go backwards (e.g., coarse seconds resolution, clock adjustments, or out-of-order delivery), the WI_ASSERT can fire in debug builds.

Either rely only on delivery order (remove the timestamp-order claim/assert), or switch to a truly monotonic ordering field and update Get() accordingly. The simplest fix is to drop the timestamp-order assumption in Append().

    // Events are recorded in Docker's delivery order, which is also timestamp order. Subscribers rely on
    // this: they resume from a sequence number, so an out-of-order event could never be inserted where it
    // belongs without hiding it from readers that already moved past that point.
    WI_ASSERT(m_events.empty() || m_events.back().time <= Event.time);

[Copilot]

Pull request overview

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

Suppressed comments (1)

src/windows/wslcsession/EventStore.cpp:193

• const auto event = GetLockHeld(...).value(); incurs an extra deep copy of Event (and its Attributes map): once into the optional returned by GetLockHeld(), then again into event. You can keep the optional in a local and std::move its value to avoid that second copy.

        const auto event = GetLockHeld(SequenceNumber.value()).value();

[Pooja Trivedi (ptrivedi)]

does GetNext() need to be serialized with a per-stream mutex or something? I am thinking concurrent GetNext() calls may end up corrupting the stream cursor? m_lock here will help preventing the cursor from being mutated simultaneously, but if two calls end up passing the same curosr value to WaitForEvent above, there could be a race and one of the streams could fail and cursot would be inconsistent

[Pooja Trivedi (ptrivedi)]

Hmm, so the m_pendingCreate will only get cleared when the docker create event is processed.. but if that fails for whatever reason, it will wedge all the later creates? this shoudl be time-bound

[Pooja Trivedi (ptrivedi)]

pretty large positive seconds could still overflow when wait_until() converts sys_seconds to the windows system_clock?

[Pooja Trivedi (ptrivedi)]

nit: doesn't seem like this include would be needed here.

[Pooja Trivedi (ptrivedi)]

nit: should double-check these are present in error codes documentation and in g_commonErrors

[Pooja Trivedi (ptrivedi)]

this 10s timeout does not bound the test, it will still go ahead and call future.get() and hang?

[Pooja Trivedi (ptrivedi)]

we should likely have a test for what happens when a stream falls behind, given the store retains only 256 events

[Copilot]

🟡 Changes recommended

WSLCContainerImpl::OnEvent records Kill events without taking m_lock, creating a potential data race with other container operations/teardown.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/windows/wslcsession/WSLCContainer.cpp:1271

• OnEvent() handles ContainerEvent::Kill by calling RecordEvent() without taking m_lock, but RecordEvent() reads members (e.g., m_labels, m_name, m_image, m_id) that are otherwise accessed under m_lock (e.g., GetLabels). This introduces a potential data race with concurrent state/lifecycle operations or teardown.

    if (event == ContainerEvent::Kill)
    {
        RecordEvent("kill", eventTime);
        return;
    }

• Files reviewed: 19/19 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces a new COM API surface plus new concurrency-sensitive session/container eventing paths that warrant final human review despite only minor actionable nits.

Review details

Suppressed comments (2)

src/windows/wslcsession/EventStore.cpp:196

• EventStore::Get() copies the entire Event (including the potentially large Actor.Attributes map) on every iteration via GetLockHeld(...).value(), even for events that are skipped by the time window or filters. This can add avoidable CPU/memory churn for busy streams.

Consider reading the buffered event by reference under m_lock and only copying when returning a matched event.

        const auto event = GetLockHeld(SequenceNumber.value()).value();
        const std::chrono::sys_seconds eventTime{std::chrono::seconds{event.time}};

test/windows/WSLCTests.cpp:7023

• Typo in comment: “unhandling init process” should be “unhandled init process”.

        // Docker emits a 'kill' event per signal. SIGWINCH is ignored by an unhandling init process, so the
        // container keeps running and each signal costs only one event.

• Files reviewed: 19/19 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces a new COM surface plus multi-threaded event-streaming and ordering coordination that should receive final human review for correctness and long-term supportability.

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite