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

Fixes #468 — race condition for Observable in concatenation operators #469

Merged
merged 13 commits into from Jan 20, 2018

Conversation

Projects
None yet
1 participant
@alexandru
Member

alexandru commented Nov 21, 2017

Fixes #468.

The story is that there's a concurrency problem on cancellation, the contract being relaxed due to usage of lazySet and, in the case of ++, due to a missing async boundary.

Thus this PR fixes, for Observable, the implementations of:

  • concat
  • concatMap
  • mapTask
  • scanTask
  • flatScan

As part of this ticket, due to needs of testing, I've also introduced observable.uncancelable to mirror the similar operation on Task.

Slight note on performance — due to avoiding extra synchronization by using a second atomic reference and due to other improvements, the performance is actually slightly better than in 2.3.2.

A port for 2.3.2 will follow shortly.

@alexandru alexandru changed the title from Fixes #468 — race condition in ConcatObservable to WIP: Fixes #468 — race condition in ConcatObservable Nov 21, 2017

@codecov

This comment has been minimized.

codecov bot commented Nov 21, 2017

Codecov Report

Merging #469 into master will increase coverage by 0.05%.
The diff coverage is 85.97%.

@@            Coverage Diff             @@
##           master     #469      +/-   ##
==========================================
+ Coverage   90.43%   90.48%   +0.05%     
==========================================
  Files         366      367       +1     
  Lines        9657     9726      +69     
  Branches     1829     1854      +25     
==========================================
+ Hits         8733     8801      +68     
- Misses        924      925       +1

alexandru added some commits Jan 14, 2018

@alexandru alexandru changed the title from WIP: Fixes #468 — race condition in ConcatObservable to Fixes #468 — race condition for Observable in concatenation operators Jan 20, 2018

alexandru added some commits Jan 20, 2018

@alexandru alexandru added this to the 3.0.0 milestone Jan 20, 2018

@alexandru alexandru self-assigned this Jan 20, 2018

alexandru added some commits Jan 20, 2018

@alexandru alexandru merged commit 2966133 into monix:master Jan 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment