Skip to content

Commit 4851637

Browse files
akarnokdakarnokd
authored
2.x: add safeguards to generate() (#4932)
* 2.x: add safeguards to generate() * Fix typo in the test name
1 parent a902d4a commit 4851637

File tree

4 files changed

+157
-23
lines changed

4 files changed

+157
-23
lines changed

‎src/main/java/io/reactivex/internal/operators/flowable/FlowableGenerate.java

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ static final class GeneratorSubscription<T, S>
6868

6969
boolean terminate;
7070

71+
boolean hasNext;
72+
7173
GeneratorSubscription(Subscriber<? super T> actual,
7274
BiFunction<S, ? super Emitter<T>, S> generator,
7375
Consumer<? super S> disposeState, S initialState) {
@@ -96,21 +98,27 @@ public void request(long n) {
9698
while (e != n) {
9799

98100
if (cancelled) {
101+
state = null;
99102
dispose(s);
100103
return;
101104
}
102105

106+
hasNext = false;
107+
103108
try {
104109
s = f.apply(s, this);
105110
} catch (Throwable ex) {
106111
Exceptions.throwIfFatal(ex);
107112
cancelled = true;
108-
actual.onError(ex);
113+
state = null;
114+
onError(ex);
115+
dispose(s);
109116
return;
110117
}
111118

112119
if (terminate) {
113120
cancelled = true;
121+
state = null;
114122
dispose(s);
115123
return;
116124
}
@@ -146,33 +154,48 @@ public void cancel() {
146154

147155
// if there are no running requests, just dispose the state
148156
if (BackpressureHelper.add(this, 1) == 0) {
149-
dispose(state);
157+
S s = state;
158+
state = null;
159+
dispose(s);
150160
}
151161
}
152162
}
153163

154164
@Override
155165
public void onNext(T t) {
156-
if (t == null) {
157-
onError(new NullPointerException("onNext called with null. Null values are generally not allowed in 2.x operators and sources."));
158-
return;
166+
if (!terminate) {
167+
if (hasNext) {
168+
onError(new IllegalStateException("onNext already called in this generate turn"));
169+
} else {
170+
if (t == null) {
171+
onError(new NullPointerException("onNext called with null. Null values are generally not allowed in 2.x operators and sources."));
172+
} else {
173+
hasNext = true;
174+
actual.onNext(t);
175+
}
176+
}
159177
}
160-
actual.onNext(t);
161178
}
162179

163180
@Override
164181
public void onError(Throwable t) {
165-
if (t == null) {
166-
t = new NullPointerException("onError called with null. Null values are generally not allowed in 2.x operators and sources.");
182+
if (terminate) {
183+
RxJavaPlugins.onError(t);
184+
} else {
185+
if (t == null) {
186+
t = new NullPointerException("onError called with null. Null values are generally not allowed in 2.x operators and sources.");
187+
}
188+
terminate = true;
189+
actual.onError(t);
167190
}
168-
terminate = true;
169-
actual.onError(t);
170191
}
171192

172193
@Override
173194
public void onComplete() {
174-
terminate = true;
175-
actual.onComplete();
195+
if (!terminate) {
196+
terminate = true;
197+
actual.onComplete();
198+
}
176199
}
177200
}
178201
}

‎src/main/java/io/reactivex/internal/operators/observable/ObservableGenerate.java

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ static final class GeneratorDisposable<T, S>
6464

6565
boolean terminate;
6666

67+
boolean hasNext;
68+
6769
GeneratorDisposable(Observer<? super T> actual,
6870
BiFunction<S, ? super Emitter<T>, S> generator,
6971
Consumer<? super S> disposeState, S initialState) {
@@ -92,13 +94,16 @@ public void run() {
9294
return;
9395
}
9496

97+
hasNext = false;
98+
9599
try {
96100
s = f.apply(s, this);
97101
} catch (Throwable ex) {
98102
Exceptions.throwIfFatal(ex);
99103
state = null;
100104
cancelled = true;
101-
actual.onError(ex);
105+
onError(ex);
106+
dispose(s);
102107
return;
103108
}
104109

@@ -131,28 +136,42 @@ public boolean isDisposed() {
131136
return cancelled;
132137
}
133138

139+
134140
@Override
135141
public void onNext(T t) {
136-
if (t == null) {
137-
onError(new NullPointerException("onNext called with null. Null values are generally not allowed in 2.x operators and sources."));
138-
return;
142+
if (!terminate) {
143+
if (hasNext) {
144+
onError(new IllegalStateException("onNext already called in this generate turn"));
145+
} else {
146+
if (t == null) {
147+
onError(new NullPointerException("onNext called with null. Null values are generally not allowed in 2.x operators and sources."));
148+
} else {
149+
hasNext = true;
150+
actual.onNext(t);
151+
}
152+
}
139153
}
140-
actual.onNext(t);
141154
}
142155

143156
@Override
144157
public void onError(Throwable t) {
145-
if (t == null) {
146-
t = new NullPointerException("onError called with null. Null values are generally not allowed in 2.x operators and sources.");
158+
if (terminate) {
159+
RxJavaPlugins.onError(t);
160+
} else {
161+
if (t == null) {
162+
t = new NullPointerException("onError called with null. Null values are generally not allowed in 2.x operators and sources.");
163+
}
164+
terminate = true;
165+
actual.onError(t);
147166
}
148-
terminate = true;
149-
actual.onError(t);
150167
}
151168

152169
@Override
153170
public void onComplete() {
154-
terminate = true;
155-
actual.onComplete();
171+
if (!terminate) {
172+
terminate = true;
173+
actual.onComplete();
174+
}
156175
}
157176
}
158177
}

‎src/test/java/io/reactivex/internal/operators/flowable/FlowableGenerateTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,50 @@ public void run() {
236236
ts.assertValueCount(1000);
237237
}
238238
}
239+
240+
@Test
241+
public void multipleOnNext() {
242+
Flowable.generate(new Consumer<Emitter<Object>>() {
243+
@Override
244+
public void accept(Emitter<Object> e) throws Exception {
245+
e.onNext(1);
246+
e.onNext(2);
247+
}
248+
})
249+
.test(1)
250+
.assertFailure(IllegalStateException.class, 1);
251+
}
252+
253+
@Test
254+
public void multipleOnError() {
255+
List<Throwable> errors = TestHelper.trackPluginErrors();
256+
try {
257+
Flowable.generate(new Consumer<Emitter<Object>>() {
258+
@Override
259+
public void accept(Emitter<Object> e) throws Exception {
260+
e.onError(new TestException("First"));
261+
e.onError(new TestException("Second"));
262+
}
263+
})
264+
.test(1)
265+
.assertFailure(TestException.class);
266+
267+
TestHelper.assertError(errors, 0, TestException.class, "Second");
268+
} finally {
269+
RxJavaPlugins.reset();
270+
}
271+
}
272+
273+
@Test
274+
public void multipleOnComplete() {
275+
Flowable.generate(new Consumer<Emitter<Object>>() {
276+
@Override
277+
public void accept(Emitter<Object> e) throws Exception {
278+
e.onComplete();
279+
e.onComplete();
280+
}
281+
})
282+
.test(1)
283+
.assertResult();
284+
}
239285
}

‎src/test/java/io/reactivex/internal/operators/observable/ObservableGenerateTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,4 +147,50 @@ public void accept(Integer s, Emitter<Object> e) throws Exception {
147147

148148
assertEquals(0, call[0]);
149149
}
150+
151+
@Test
152+
public void multipleOnNext() {
153+
Observable.generate(new Consumer<Emitter<Object>>() {
154+
@Override
155+
public void accept(Emitter<Object> e) throws Exception {
156+
e.onNext(1);
157+
e.onNext(2);
158+
}
159+
})
160+
.test()
161+
.assertFailure(IllegalStateException.class, 1);
162+
}
163+
164+
@Test
165+
public void multipleOnError() {
166+
List<Throwable> errors = TestHelper.trackPluginErrors();
167+
try {
168+
Observable.generate(new Consumer<Emitter<Object>>() {
169+
@Override
170+
public void accept(Emitter<Object> e) throws Exception {
171+
e.onError(new TestException("First"));
172+
e.onError(new TestException("Second"));
173+
}
174+
})
175+
.test()
176+
.assertFailure(TestException.class);
177+
178+
TestHelper.assertError(errors, 0, TestException.class, "Second");
179+
} finally {
180+
RxJavaPlugins.reset();
181+
}
182+
}
183+
184+
@Test
185+
public void multipleOnComplete() {
186+
Observable.generate(new Consumer<Emitter<Object>>() {
187+
@Override
188+
public void accept(Emitter<Object> e) throws Exception {
189+
e.onComplete();
190+
e.onComplete();
191+
}
192+
})
193+
.test()
194+
.assertResult();
195+
}
150196
}

0 commit comments

Comments
 (0)