Skip to content

Fix retryWhen/repeatWhen potential concurrent reentry when resubscribing - #4241

Merged
akarnokd merged 1 commit into
ReactiveX:1.xfrom
akarnokd:RedoConcurrentSignalFix
Jul 27, 2016
Merged

Fix retryWhen/repeatWhen potential concurrent reentry when resubscribing#4241
akarnokd merged 1 commit into
ReactiveX:1.xfrom
akarnokd:RedoConcurrentSignalFix

Conversation

@akarnokd

Copy link
Copy Markdown
Member

When the function's returned Observable signals the resubscription in the operator redo (which is the base for repeatWhen and retryWhen). It is possible an asyncronous source triggers the function (and thus the same Observable) from another thread while the first is still coming back from the resubscription itself.

This PR serializes the dispatching BehaviorSubject to prevent such concurrent execution of the when chain.

This may or may not be the source of the failure of #4175 but I can't reproduce the failure with the 1.x branch. /cc @davidmoten.

@akarnokd akarnokd added the Bug label Jul 25, 2016
@akarnokd akarnokd added this to the 1.2 milestone Jul 25, 2016
@codecov-io

Copy link
Copy Markdown

Current coverage is 84.23% (diff: 100%)

Merging #4241 into 1.x will increase coverage by 0.02%

@@                1.x      #4241   diff @@
==========================================
  Files           265        265          
  Lines         17313      17313          
  Methods           0          0          
  Messages          0          0          
  Branches       2627       2627          
==========================================
+ Hits          14578      14583     +5   
+ Misses         1885       1880     -5   
  Partials        850        850          

Powered by Codecov. Last update 45f6072...05ca826

@artem-zinnatullin

Copy link
Copy Markdown
Contributor

👍

@davidmoten

Copy link
Copy Markdown
Collaborator

Worth a shot, gnarly bug 👍

@akarnokd
akarnokd merged commit d6f395e into ReactiveX:1.x Jul 27, 2016
@akarnokd
akarnokd deleted the RedoConcurrentSignalFix branch July 27, 2016 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants