[DO NOT MERGE] Validation build: fail-fast on IContextCallback dispatch recursion#2464
Closed
Sergio0694 wants to merge 1 commit into
Closed
[DO NOT MERGE] Validation build: fail-fast on IContextCallback dispatch recursion#2464Sergio0694 wants to merge 1 commit into
Sergio0694 wants to merge 1 commit into
Conversation
Instruments the 'IContextCallbackVftbl.ContextCallback' dispatch with a thread-static depth counter that fails fast if the dispatch is ever reentered on the same thread. This validates the concern raised in PR #1865 about whether the thread-static callback state approach could be reentered (e.g. via STA message pumping), to measure how common (if ever) this is in real world scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
What this is
A throwaway validation build to answer the open question from #1865: can the
IContextCallbackdispatch ever be reentered on the same thread? (jkotas: "Are you sure that there cannot ever be recursion that would cause multiple recursive uses ofCallbackDataon the same thread?")This instruments the existing dispatch in
ABI.WinRT.Interop.IContextCallbackVftbl.ContextCallbackwith a[ThreadStatic]depth counter. If the dispatch is entered while a previous call on the same thread is still in progress (the reentrancy window is the nativeContextCallback_4call, e.g. due to STA message pumping), it callsEnvironment.FailFastwith a useful message instead of silently continuing.The goal — per this comment — is to ship a build with this assertion and run it through our tests and the Store to see how common (if ever) this recursion actually happens in real-world scenarios. If it never fires, then the thread-static state optimization from #1865 (without a slower fallback) would be safe.
Notes
Environment.FailFast(notDebug.Assert) so it fires in Release/Store builds too.finallyso a legitimate later call on the same thread isn't misdetected if the dispatch throws.#if NET && CsWinRT_LANG_11_FEATURESfast path (net8.0/net9.0) is instrumented — that's the path that matters for the Store.IContextCallbackVftblisinternal).Not intended to merge — opened to get a preview package from CI.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com