Skip to content

fix: close first-read race in RxState, RxSuspension and RxCacheSize - #4519

Merged
glennawatson merged 1 commit into
mainfrom
fix/rxstate-exception-handler-race
Sep 26, 2026
Merged

glennawatson merged 1 commit into
mainfrom
fix/rxstate-exception-handler-race

Conversation

@glennawatson

Copy link
Copy Markdown
Contributor

Summary

RxState.DefaultExceptionHandler, RxSuspension.SuspensionHost and the RxCacheSize limits never return an unset value, however many threads read them first.

  • RxState.DefaultExceptionHandler never returns null. The handler is published once, atomically, and the first handler published (the builder's or the default) wins, as before.
  • RxSuspension.SuspensionHost never returns null. The host is published once under a lock, so only one default host is created.
  • RxCacheSize.SmallCacheLimit and BigCacheLimit never return 0. The two limits are published together, so a reader never sees one set and the other not.
  • A null handler or host passed to the internal initializers no longer poisons the state. The argument is rejected before anything changes, so the default still applies.

Why

Each of these set its "initialized" flag before it assigned the value, so a thread racing the first read could see the flag and return the unassigned field.

  • Code generated by ReactiveUI.SourceGenerators calls RxState.DefaultExceptionHandler!.OnNext(...), which dereferences that null.
  • The fix covers both the lean and .Reactive flavours, which compile the same ReactiveUI.Shared source.

Closes #4517

Breaking changes

None

How this was verified

New tests race several threads on the first read of each value over many resets, and cover the builder, default and null-argument paths.

  • The race and null-argument tests reproduce the defect on main. They run in both the lean and .Reactive test assemblies.

Notes for the reviewer

The three files under src/ReactiveUI.Shared are the whole fix; the value field is now its own initialization flag.

  • RxSuspension uses a lock rather than compare-and-exchange so that a losing thread never builds a SuspensionHost (a disposable ReactiveObject) only to throw it away.
  • RxCacheSize holds both limits in a private immutable CacheLimits object so they publish in one step.
  • FirstReadRace in the test infrastructure is the shared barrier harness the three race tests use.

Checklist

  • I have read the Contribute guide
  • The PR title follows Conventional Commits
  • Tests cover this change, or the summary says why they do not
  • New or changed public API has XML documentation

RxState.DefaultExceptionHandler, RxSuspension.SuspensionHost and the RxCacheSize
limits set an "initialized" flag before assigning the value, so a thread racing
the first read could see the flag and return null (or 0 for the cache limits).
Passing null to the internal initializers also left the flag set and the value
null for good.

Each value is now its own initialization flag: it is read with Volatile.Read and
published once, with Interlocked.CompareExchange for the handler and the cache
limits (which now travel together in one immutable object) and under a lock for
the suspension host so only one default host is created. The first value
published still wins, and null arguments are rejected before any state changes.

Closes #4517
@codecov

codecov Bot commented Sep 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.68%. Comparing base (aba5bb2) to head (dc2fa43).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4519      +/-   ##
==========================================
+ Coverage   93.62%   93.68%   +0.05%     
==========================================
  Files         175      175              
  Lines        6227     6220       -7     
  Branches      743      741       -2     
==========================================
- Hits         5830     5827       -3     
+ Misses        396      392       -4     
  Partials        1        1              

☔ 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.

@sonarqubecloud

Copy link
Copy Markdown

@glennawatson
glennawatson merged commit a4baf2a into main Sep 26, 2026
17 of 18 checks passed
@glennawatson
glennawatson deleted the fix/rxstate-exception-handler-race branch September 26, 2026 15:50
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.

RxState.DefaultExceptionHandler returns null when several threads read it for the first time

1 participant