Add docs on Windows Runtime events infrastructure#2320
Add docs on Windows Runtime events infrastructure#2320Sergio0694 wants to merge 2 commits intostaging/3.0from
Conversation
Add comprehensive documentation for CsWinRT's event infrastructure under docs/event-infrastructure.md. Covers core types (EventRegistrationToken, EventSource<T>, EventSourceState<T>, EventSourceCache, EventRegistrationTokenTable<T>), RCW (managed→native) and CCW (native→managed) event flows, lifetime and GC semantics (including reference tracking and cleanup), and an in-depth analysis of static event lifetime issues and potential fixes. Intended to help maintainers and contributors understand event registration, marshalling, and cross-context pitfalls.
There was a problem hiding this comment.
Pull request overview
Adds a new maintainer-focused document explaining CsWinRT (WinRT.Runtime2) Windows Runtime event infrastructure, including RCW/CCW event flows, caching/lifetime behavior, and static event context-switch pitfalls.
Changes:
- Add comprehensive documentation for core event infrastructure types (
EventSource<T>,EventSourceState<T>,EventSourceCache,EventRegistrationToken(… ),EventRegistrationTokenTable<T>). - Document end-to-end RCW (managed→native) and CCW (native→managed) subscription/unsubscription flows with diagrams.
- Analyze static event lifetime issues when activation factory caches are refreshed across COM context switches, and discuss potential mitigation approaches.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **On `Subscribe`**: After creating a new `EventSourceState<T>`, `EventSourceCache.Create(...)` is called to store a weak reference to the state. This only works if the native object supports `IWeakReferenceSource`. | ||
| - **On `EventSource<T>` construction**: The constructor calls `EventSourceCache.GetState(...)` to check if there's an existing state for this object+event. If so, it reconnects to it. | ||
| - **On `EventSourceState<T>` disposal/finalization**: `EventSourceCache.Remove(...)` is called to clean up. | ||
|
|
There was a problem hiding this comment.
This bullet implies EventSourceState<T> disposal/finalization cleans up EventSourceCache entries. In the current implementation, EventSourceState<T>.OnDispose() sets _thisPtr to null and then calls EventSourceCache.Remove(_thisPtr, ...), which passes a null pointer and effectively skips cache removal. Please either update this doc to describe the current behavior accurately, or call out that cache removal is the intended design and depends on fixing OnDispose() to pass the saved pointer value.
| When an `EventSourceState<T>` is finalized (because the native object released its CCW reference and nothing else references the state): | ||
|
|
||
| 1. The finalizer calls `OnDispose()`. | ||
| 2. `OnDispose()` calls `EventSourceCache.Remove(...)` to clean up the cache entry. | ||
| 3. If this was the last state in the cache for that native object, the `EventSourceCache` entry itself is removed from the global dictionary. | ||
|
|
||
| Explicit disposal (via `Unsubscribe`) follows the same path but also calls `GC.SuppressFinalize` to avoid double cleanup. |
There was a problem hiding this comment.
This GC cleanup description assumes OnDispose() removes the associated entry from EventSourceCache. In EventSourceState<T> today, OnDispose() nulls _thisPtr before invoking EventSourceCache.Remove(_thisPtr, ...), so cache removal does not actually occur. Please adjust the description (or add a note) so it matches the current runtime behavior.
| For example, one might consider using a **static (codegen-level) event source field** (similar to how `WindowsRuntimeObservableVector<T>` stores its event source in a per-instance field). But this has the same problem: the static field would be populated on first access with an `EventSource<T>` wrapping the activation factory from that context. After a context switch, that event source would wrap a stale object reference for a different context, making native vtable calls through it invalid. Adding the same `IsInCurrentContext` invalidation pattern to the field would bring us right back to the original problem — replacing the event source loses the state. | ||
|
|
||
| #### Why Alternative Cache Key Strategies Don't Help | ||
|
|
||
| One might consider using a **stable, context-independent key** (e.g., a singleton per type or a type handle) instead of the activation factory object reference. This would prevent the `ConditionalWeakTable` entry from being lost on context switches. However, it introduces a different problem: the `EventSource<T>` cached against that stable key permanently wraps the `WindowsRuntimeObjectReference` from the original context. After a context switch, `Subscribe`/`Unsubscribe` on that event source would call into the native vtable through a stale, wrong-context object reference. | ||
|
|
||
| Similarly, a **static (codegen-level) event source field** (similar to how `WindowsRuntimeObservableVector<T>` stores its event source in a per-instance field) would be populated on first access with an `EventSource<T>` wrapping the activation factory from that context. After a context switch, that event source would wrap a stale object reference for a different context. Adding the same `IsInCurrentContext` invalidation pattern to the field would bring us right back to the original problem — replacing the event source loses the state. | ||
|
|
There was a problem hiding this comment.
The paragraph about using a static event source field is repeated (here and again a few lines later), and the example is inaccurate: WindowsRuntimeObservableVector<T> does not store an event source in a per-instance field (it routes through TIObservableVectorMethods.VectorChanged(...).Subscribe/Unsubscribe). Please deduplicate this explanation and either remove or replace the WindowsRuntimeObservableVector<T> reference with an example that matches the codebase.
Update the mermaid diagram in docs/event-infrastructure.md to make the static field explicit. The changes add separate __IStaticsType nodes for "before" and "after" states, split the activation factory labels into distinct nodes, and simplify the "strong ref (field)" arrow label to "strong ref". This improves clarity of ownership/reference relationships in the event infrastructure diagram without changing substantive content.
Add comprehensive documentation for CsWinRT's event infrastructure under docs/event-infrastructure.md. Covers core types (EventRegistrationToken, EventSource, EventSourceState, EventSourceCache, EventRegistrationTokenTable), RCW (managed→native) and CCW (native→managed) event flows, lifetime and GC semantics (including reference tracking and cleanup), and an in-depth analysis of static event lifetime issues and potential fixes. Intended to help maintainers and contributors understand event registration, marshalling, and cross-context pitfalls.