Skip to content

Commit d40f923

Browse files
akarnokdakarnokd
authored
2.x: Don't dispose the winner of {Single|Maybe|Completable}.amb() (#6375)
* 2.x: Don't dispose the winner of {Single|Maybe|Completable}.amb() * Add null-source test to MaybeAmbTest
1 parent 5106a20 commit d40f923

File tree

8 files changed

+462
-50
lines changed

8 files changed

+462
-50
lines changed

‎src/main/java/io/reactivex/internal/operators/completable/CompletableAmb.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ public void subscribeActual(final CompletableObserver observer) {
6363

6464
final AtomicBoolean once = new AtomicBoolean();
6565

66-
CompletableObserver inner = new Amb(once, set, observer);
67-
6866
for (int i = 0; i < count; i++) {
6967
CompletableSource c = sources[i];
7068
if (set.isDisposed()) {
@@ -82,7 +80,7 @@ public void subscribeActual(final CompletableObserver observer) {
8280
}
8381

8482
// no need to have separate subscribers because inner is stateless
85-
c.subscribe(inner);
83+
c.subscribe(new Amb(once, set, observer));
8684
}
8785

8886
if (count == 0) {
@@ -91,9 +89,14 @@ public void subscribeActual(final CompletableObserver observer) {
9189
}
9290

9391
static final class Amb implements CompletableObserver {
94-
private final AtomicBoolean once;
95-
private final CompositeDisposable set;
96-
private final CompletableObserver downstream;
92+
93+
final AtomicBoolean once;
94+
95+
final CompositeDisposable set;
96+
97+
final CompletableObserver downstream;
98+
99+
Disposable upstream;
97100

98101
Amb(AtomicBoolean once, CompositeDisposable set, CompletableObserver observer) {
99102
this.once = once;
@@ -104,6 +107,7 @@ static final class Amb implements CompletableObserver {
104107
@Override
105108
public void onComplete() {
106109
if (once.compareAndSet(false, true)) {
110+
set.delete(upstream);
107111
set.dispose();
108112
downstream.onComplete();
109113
}
@@ -112,6 +116,7 @@ public void onComplete() {
112116
@Override
113117
public void onError(Throwable e) {
114118
if (once.compareAndSet(false, true)) {
119+
set.delete(upstream);
115120
set.dispose();
116121
downstream.onError(e);
117122
} else {
@@ -121,8 +126,8 @@ public void onError(Throwable e) {
121126

122127
@Override
123128
public void onSubscribe(Disposable d) {
129+
upstream = d;
124130
set.add(d);
125131
}
126-
127132
}
128133
}

‎src/main/java/io/reactivex/internal/operators/maybe/MaybeAmb.java‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -64,64 +64,63 @@ protected void subscribeActual(MaybeObserver<? super T> observer) {
6464
count = sources.length;
6565
}
6666

67-
AmbMaybeObserver<T> parent = new AmbMaybeObserver<T>(observer);
68-
observer.onSubscribe(parent);
67+
CompositeDisposable set = new CompositeDisposable();
68+
observer.onSubscribe(set);
69+
70+
AtomicBoolean winner = new AtomicBoolean();
6971

7072
for (int i = 0; i < count; i++) {
7173
MaybeSource<? extends T> s = sources[i];
72-
if (parent.isDisposed()) {
74+
if (set.isDisposed()) {
7375
return;
7476
}
7577

7678
if (s == null) {
77-
parent.onError(new NullPointerException("One of the MaybeSources is null"));
79+
set.dispose();
80+
NullPointerException ex = new NullPointerException("One of the MaybeSources is null");
81+
if (winner.compareAndSet(false, true)) {
82+
observer.onError(ex);
83+
} else {
84+
RxJavaPlugins.onError(ex);
85+
}
7886
return;
7987
}
8088

81-
s.subscribe(parent);
89+
s.subscribe(new AmbMaybeObserver<T>(observer, set, winner));
8290
}
8391

8492
if (count == 0) {
8593
observer.onComplete();
8694
}
87-
8895
}
8996

9097
static final class AmbMaybeObserver<T>
91-
extends AtomicBoolean
92-
implements MaybeObserver<T>, Disposable {
93-
94-
private static final long serialVersionUID = -7044685185359438206L;
98+
implements MaybeObserver<T> {
9599

96100
final MaybeObserver<? super T> downstream;
97101

98-
final CompositeDisposable set;
102+
final AtomicBoolean winner;
99103

100-
AmbMaybeObserver(MaybeObserver<? super T> downstream) {
101-
this.downstream = downstream;
102-
this.set = new CompositeDisposable();
103-
}
104+
final CompositeDisposable set;
104105

105-
@Override
106-
public void dispose() {
107-
if (compareAndSet(false, true)) {
108-
set.dispose();
109-
}
110-
}
106+
Disposable upstream;
111107

112-
@Override
113-
public boolean isDisposed() {
114-
return get();
108+
AmbMaybeObserver(MaybeObserver<? super T> downstream, CompositeDisposable set, AtomicBoolean winner) {
109+
this.downstream = downstream;
110+
this.set = set;
111+
this.winner = winner;
115112
}
116113

117114
@Override
118115
public void onSubscribe(Disposable d) {
116+
upstream = d;
119117
set.add(d);
120118
}
121119

122120
@Override
123121
public void onSuccess(T value) {
124-
if (compareAndSet(false, true)) {
122+
if (winner.compareAndSet(false, true)) {
123+
set.delete(upstream);
125124
set.dispose();
126125

127126
downstream.onSuccess(value);
@@ -130,7 +129,8 @@ public void onSuccess(T value) {
130129

131130
@Override
132131
public void onError(Throwable e) {
133-
if (compareAndSet(false, true)) {
132+
if (winner.compareAndSet(false, true)) {
133+
set.delete(upstream);
134134
set.dispose();
135135

136136
downstream.onError(e);
@@ -141,12 +141,12 @@ public void onError(Throwable e) {
141141

142142
@Override
143143
public void onComplete() {
144-
if (compareAndSet(false, true)) {
144+
if (winner.compareAndSet(false, true)) {
145+
set.delete(upstream);
145146
set.dispose();
146147

147148
downstream.onComplete();
148149
}
149150
}
150-
151151
}
152152
}

‎src/main/java/io/reactivex/internal/operators/single/SingleAmb.java‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,61 +59,67 @@ protected void subscribeActual(final SingleObserver<? super T> observer) {
5959
count = sources.length;
6060
}
6161

62+
final AtomicBoolean winner = new AtomicBoolean();
6263
final CompositeDisposable set = new CompositeDisposable();
6364

64-
AmbSingleObserver<T> shared = new AmbSingleObserver<T>(observer, set);
6565
observer.onSubscribe(set);
6666

6767
for (int i = 0; i < count; i++) {
6868
SingleSource<? extends T> s1 = sources[i];
69-
if (shared.get()) {
69+
if (set.isDisposed()) {
7070
return;
7171
}
7272

7373
if (s1 == null) {
7474
set.dispose();
7575
Throwable e = new NullPointerException("One of the sources is null");
76-
if (shared.compareAndSet(false, true)) {
76+
if (winner.compareAndSet(false, true)) {
7777
observer.onError(e);
7878
} else {
7979
RxJavaPlugins.onError(e);
8080
}
8181
return;
8282
}
8383

84-
s1.subscribe(shared);
84+
s1.subscribe(new AmbSingleObserver<T>(observer, set, winner));
8585
}
8686
}
8787

88-
static final class AmbSingleObserver<T> extends AtomicBoolean implements SingleObserver<T> {
89-
90-
private static final long serialVersionUID = -1944085461036028108L;
88+
static final class AmbSingleObserver<T> implements SingleObserver<T> {
9189

9290
final CompositeDisposable set;
9391

9492
final SingleObserver<? super T> downstream;
9593

96-
AmbSingleObserver(SingleObserver<? super T> observer, CompositeDisposable set) {
94+
final AtomicBoolean winner;
95+
96+
Disposable upstream;
97+
98+
AmbSingleObserver(SingleObserver<? super T> observer, CompositeDisposable set, AtomicBoolean winner) {
9799
this.downstream = observer;
98100
this.set = set;
101+
this.winner = winner;
99102
}
100103

101104
@Override
102105
public void onSubscribe(Disposable d) {
106+
this.upstream = d;
103107
set.add(d);
104108
}
105109

106110
@Override
107111
public void onSuccess(T value) {
108-
if (compareAndSet(false, true)) {
112+
if (winner.compareAndSet(false, true)) {
113+
set.delete(upstream);
109114
set.dispose();
110115
downstream.onSuccess(value);
111116
}
112117
}
113118

114119
@Override
115120
public void onError(Throwable e) {
116-
if (compareAndSet(false, true)) {
121+
if (winner.compareAndSet(false, true)) {
122+
set.delete(upstream);
117123
set.dispose();
118124
downstream.onError(e);
119125
} else {

‎src/test/java/io/reactivex/internal/operators/completable/CompletableAmbTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,21 @@
1616
import static org.junit.Assert.*;
1717

1818
import java.util.*;
19+
import java.util.concurrent.*;
1920
import java.util.concurrent.atomic.AtomicBoolean;
2021

2122
import org.junit.Test;
2223

2324
import io.reactivex.*;
2425
import io.reactivex.disposables.*;
2526
import io.reactivex.exceptions.TestException;
27+
import io.reactivex.functions.*;
28+
import io.reactivex.internal.functions.Functions;
2629
import io.reactivex.internal.operators.completable.CompletableAmb.Amb;
2730
import io.reactivex.observers.TestObserver;
2831
import io.reactivex.plugins.RxJavaPlugins;
2932
import io.reactivex.processors.PublishProcessor;
33+
import io.reactivex.schedulers.Schedulers;
3034
import io.reactivex.subjects.*;
3135

3236
public class CompletableAmbTest {
@@ -173,6 +177,7 @@ public void ambRace() {
173177
CompositeDisposable cd = new CompositeDisposable();
174178
AtomicBoolean once = new AtomicBoolean();
175179
Amb a = new Amb(once, cd, to);
180+
a.onSubscribe(Disposables.empty());
176181

177182
a.onComplete();
178183
a.onComplete();
@@ -259,4 +264,54 @@ public void untilCompletableOtherError() {
259264
to.assertFailure(TestException.class);
260265
}
261266

267+
@Test
268+
public void noWinnerErrorDispose() throws Exception {
269+
final TestException ex = new TestException();
270+
for (int i = 0; i < TestHelper.RACE_LONG_LOOPS; i++) {
271+
final AtomicBoolean interrupted = new AtomicBoolean();
272+
final CountDownLatch cdl = new CountDownLatch(1);
273+
274+
Completable.ambArray(
275+
Completable.error(ex)
276+
.subscribeOn(Schedulers.single())
277+
.observeOn(Schedulers.computation()),
278+
Completable.never()
279+
)
280+
.subscribe(Functions.EMPTY_ACTION, new Consumer<Throwable>() {
281+
@Override
282+
public void accept(Throwable e) throws Exception {
283+
interrupted.set(Thread.currentThread().isInterrupted());
284+
cdl.countDown();
285+
}
286+
});
287+
288+
assertTrue(cdl.await(500, TimeUnit.SECONDS));
289+
assertFalse("Interrupted!", interrupted.get());
290+
}
291+
}
292+
293+
@Test
294+
public void noWinnerCompleteDispose() throws Exception {
295+
for (int i = 0; i < TestHelper.RACE_LONG_LOOPS; i++) {
296+
final AtomicBoolean interrupted = new AtomicBoolean();
297+
final CountDownLatch cdl = new CountDownLatch(1);
298+
299+
Completable.ambArray(
300+
Completable.complete()
301+
.subscribeOn(Schedulers.single())
302+
.observeOn(Schedulers.computation()),
303+
Completable.never()
304+
)
305+
.subscribe(new Action() {
306+
@Override
307+
public void run() throws Exception {
308+
interrupted.set(Thread.currentThread().isInterrupted());
309+
cdl.countDown();
310+
}
311+
});
312+
313+
assertTrue(cdl.await(500, TimeUnit.SECONDS));
314+
assertFalse("Interrupted!", interrupted.get());
315+
}
316+
}
262317
}

0 commit comments

Comments
 (0)