Don't update state of PromiseCombiner when finish(null) is called (#8843)

Motivation:

When we fail a call to PromiseCombiner.finish(...) because of a null argument we must not update the internal state before throwing.

Modifications:

- First do the null check and only after we validated that the argument is not null update the internal state
- Add test case.

Modifications:

Do not mess up internal state of PromiseCombiner when finish(...) is called with a null argument.

Result:

After your change, what will change.
This commit is contained in:
Norman Maurer 2019-02-04 19:07:42 +01:00
parent db79983874
commit f6f7564602
2 changed files with 15 additions and 1 deletions

View File

@ -113,11 +113,12 @@ public final class PromiseCombiner {
* @param aggregatePromise the promise to notify when all combined futures have finished * @param aggregatePromise the promise to notify when all combined futures have finished
*/ */
public void finish(Promise<Void> aggregatePromise) { public void finish(Promise<Void> aggregatePromise) {
requireNonNull(aggregatePromise, "aggregatePromise");
if (doneAdding) { if (doneAdding) {
throw new IllegalStateException("Already finished"); throw new IllegalStateException("Already finished");
} }
doneAdding = true; doneAdding = true;
this.aggregatePromise = requireNonNull(aggregatePromise, "aggregatePromise"); this.aggregatePromise = aggregatePromise;
if (doneCount == expectedCount) { if (doneCount == expectedCount) {
tryPromise(); tryPromise();
} }

View File

@ -15,6 +15,7 @@
*/ */
package io.netty.util.concurrent; package io.netty.util.concurrent;
import org.junit.Assert;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.mockito.Mock; import org.mockito.Mock;
@ -58,6 +59,18 @@ public class PromiseCombinerTest {
combiner = new PromiseCombiner(); combiner = new PromiseCombiner();
} }
@Test
public void testNullArgument() {
try {
combiner.finish(null);
Assert.fail();
} catch (NullPointerException expected) {
// expected
}
combiner.finish(p1);
verify(p1).trySuccess(null);
}
@Test @Test
public void testNullAggregatePromise() { public void testNullAggregatePromise() {
combiner.finish(p1); combiner.finish(p1);