Skip to content

Commit

Permalink
PromiseNotifier does not propagate cancel events
Browse files Browse the repository at this point in the history
Motivation:
If the Future that the PromiseNotifier is listening to is cancelled, it does not propagate the cancel to all the promises it is expected to notify.

Modifications:
- If the future is cancelled then all the promises should be cancelled
- Add a UnaryPromiseNotifier if a collection of promises is not necessary

Result:
PromiseNotifier propagates cancel events to all promises
  • Loading branch information
Scottmitch committed Feb 19, 2016
1 parent 6e9d2bf commit 050ac70
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 7 deletions.
19 changes: 12 additions & 7 deletions common/src/main/java/io/netty/util/concurrent/PromiseNotifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,18 @@ public void operationComplete(F future) throws Exception {
logger.warn("Failed to mark a promise as success because it is done already: {}", p);
}
}
return;
}

Throwable cause = future.cause();
for (Promise<? super V> p: promises) {
if (!p.tryFailure(cause)) {
logger.warn("Failed to mark a promise as failure because it's done already: {}", p, cause);
} else if (future.isCancelled()) {
for (Promise<? super V> p: promises) {
if (!p.cancel(false)) {
logger.warn("Failed to cancel a promise because it is done already: {}", p);
}
}
} else {
Throwable cause = future.cause();
for (Promise<? super V> p: promises) {
if (!p.tryFailure(cause)) {
logger.warn("Failed to mark a promise as failure because it's done already: {}", p, cause);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright 2016 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.util.concurrent;

import io.netty.util.internal.ObjectUtil;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;

public final class UnaryPromiseNotifier<T> implements FutureListener<T> {
private static final InternalLogger logger = InternalLoggerFactory.getInstance(UnaryPromiseNotifier.class);
private final Promise<? super T> promise;

public UnaryPromiseNotifier(Promise<? super T> promise) {
this.promise = ObjectUtil.checkNotNull(promise, "promise");
}

@Override
public void operationComplete(Future<T> future) throws Exception {
cascadeTo(future, promise);
}

public static <X> void cascadeTo(Future<X> completedFuture, Promise<? super X> promise) {
if (completedFuture.isSuccess()) {
if (!promise.trySuccess(completedFuture.getNow())) {
logger.warn("Failed to mark a promise as success because it is done already: {}", promise);
}
} else if (completedFuture.isCancelled()) {
if (!promise.cancel(false)) {
logger.warn("Failed to cancel a promise because it is done already: {}", promise);
}
} else {
if (!promise.tryFailure(completedFuture.cause())) {
logger.warn("Failed to mark a promise as failure because it's done already: {}", promise,
completedFuture.cause());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ public void testListenerFailure() throws Exception {
Future<Void> future = createStrictMock(Future.class);
Throwable t = createStrictMock(Throwable.class);
expect(future.isSuccess()).andReturn(false);
expect(future.isCancelled()).andReturn(false);
expect(future.cause()).andReturn(t);
expect(p1.tryFailure(t)).andReturn(true);
expect(p2.tryFailure(t)).andReturn(true);
Expand Down

0 comments on commit 050ac70

Please sign in to comment.