Skip to content

fix: resume interaction task handlers on the captured context#4409

Merged
glennawatson merged 2 commits into
mainfrom
fix/interaction-register-handler-context
Jul 22, 2026
Merged

fix: resume interaction task handlers on the captured context#4409
glennawatson merged 2 commits into
mainfrom
fix/interaction-register-handler-context

Conversation

@glennawatson

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix.

What is the new behavior?

Interaction<TInput, TOutput>.RegisterHandler(Func<..., Task>) now resumes the async handler on the captured SynchronizationContext. A handler registered from a UI thread runs its body back on that UI context, so it can touch UI-only APIs without re-dispatching. The handler still yields off the current scheduler trampoline before running (see #4351).

What is the current behavior?

After the interaction performance refactor, the overload awaited an internal YieldToCurrentContext() with .ConfigureAwait(false). Task.Yield() posts the continuation back to the captured context, but the outer .ConfigureAwait(false) discarded it, so the handler resumed on a thread-pool thread. UI handlers then failed unless they manually marshalled back to the UI thread. Closes #4393.

What might this PR break?

None. The handler still yields before running, so the #4351 trampoline-avoidance behaviour is preserved; only the resumption thread is corrected back to the captured context, matching the documented and pre-refactor behaviour.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

  • Fix: await Task.Yield() directly instead of awaiting a Task-returning helper. Task.Yield() yields off the trampoline while resuming on the captured context; a bare Task await cannot use ConfigureAwait(false) without re-introducing the bug (and awaiting a Task without it trips the repo's ConfigureAwait analyzer). The now-dead YieldToCurrentContext helper is removed.
  • Verified by test, not reasoning: reintroducing .ConfigureAwait(false) on the yield makes the new context test fail (the handler observes a null / thread-pool context); the fix makes it pass. The Fix for 4280 interaction async handler scheduling #4351 ordering test stays green in both, confirming the yield is preserved.
  • Coverage: Interaction.cs is at 100% line (58/58) and 100% branch (8/8) coverage, measured across ReactiveUI.Tests (net9.0).
  • The new tests install a single-threaded SynchronizationContext to assert resumption on the captured context on Linux/CI; a real UI dispatcher (WPF/WinUI) exhibits the same contract but is not exercised directly here.

- RegisterHandler(Func<..., Task>) yielded off the scheduler trampoline but
  discarded the captured SynchronizationContext with ConfigureAwait(false), so
  UI handlers resumed on a thread-pool thread (#4393)
- Await Task.Yield() directly: it yields off the trampoline (#4351) while
  resuming on the captured context; drop the now-dead YieldToCurrentContext helper
- Add tests covering resumption on the installed SynchronizationContext and the
  handler still running only after the triggering subscription unwinds
@glennawatson
glennawatson marked this pull request as ready for review July 22, 2026 22:25
- Resolve Interaction.cs conflict: keep Task.Yield() (resume on captured
  SynchronizationContext, #4393) on top of main's static InvokeAsync refactor.
- Update the two new interaction tests for main's adopted analyzers:
  pass RunContinuationsAsynchronously to TaskCompletionSource, and post via
  the state-argument SynchronizationContext.Post overload (no capture).
@glennawatson
glennawatson enabled auto-merge (squash) July 22, 2026 23:09
@glennawatson
glennawatson disabled auto-merge July 22, 2026 23:30
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.13%. Comparing base (e6088d3) to head (5fa204c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4409      +/-   ##
==========================================
- Coverage   93.13%   93.13%   -0.01%     
==========================================
  Files         345      345              
  Lines       14979    14978       -1     
  Branches     1566     1566              
==========================================
- Hits        13951    13950       -1     
  Misses        755      755              
  Partials      273      273              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@glennawatson
glennawatson merged commit 2eb48f2 into main Jul 22, 2026
13 checks passed
@glennawatson
glennawatson deleted the fix/interaction-register-handler-context branch July 22, 2026 23:52
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.

[Bug]: RegisterHandler no longer resumes on the captured context

2 participants