Skip to content

3.x: Fix delay-cancellation race producing random subsequent emissions #7855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2025

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Apr 9, 2025

When a sequence using delay is cancelled asynchronously, there is a race between cancelling each individual Future inside the executor and them running, causing skips in the onNext emissions.

For example, let's say there are 3 futures waiting to be executed. The Future-1 triggers the cancellation asynchronously and emits 1. This asynchronous routine will start cancelling Future-2 and Future-3 one by one. Concurrently, the executor will look at Future-2, see it cancelled and moves onto Future-3, which is still executable as the asynchronous routine hasn't gotten to cancel Future-3. Now Future-3 runs and emits 3. This will appear to produce a sequence of [1, 3] where 2 is missing.

The fix is to hard-check for a disposed worker and prevent the onNext emissions in the operator.

Resolves #7851

@akarnokd akarnokd added this to the 3.1-support milestone Apr 9, 2025
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.62%. Comparing base (9b55d01) to head (35eb3b9).
Report is 153 commits behind head on 3.x.

Additional details and impacted files
@@             Coverage Diff              @@
##                3.x    #7855      +/-   ##
============================================
- Coverage     99.62%   99.62%   -0.01%     
  Complexity     6801     6801              
============================================
  Files           752      752              
  Lines         47707    47715       +8     
  Branches       6401     6404       +3     
============================================
+ Hits          47527    47534       +7     
+ Misses           84       80       -4     
- Partials         96      101       +5     

☔ View full report in Codecov by Sentry.
📢 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.

@akarnokd akarnokd merged commit a1b9655 into ReactiveX:3.x May 19, 2025
5 checks passed
@akarnokd akarnokd deleted the DelayFix branch May 19, 2025 07:18
Copy link
Collaborator

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Observable.delay(delayErrors=true) drops onNext signals at random when disposed of concurrently
2 participants