Fix IllegalStateException triggered by a successful connection attempt

- Fixes #1467
- Provide more information about the tasks and promises on exception for easier debugging

common/src/main/java/io/netty/util/concurrent/DefaultPromise.java

@@ -17,6 +17,7 @@ package io.netty.util.concurrent;
 
 import io.netty.util.Signal;
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.StringUtil;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
@@ -228,7 +229,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
         }
 
         if (Thread.interrupted()) {
-            throw new InterruptedException();
+            throw new InterruptedException(toString());
         }
 
         synchronized (this) {
@@ -312,7 +313,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
         }
 
         if (interruptable && Thread.interrupted()) {
-            throw new InterruptedException();
+            throw new InterruptedException(toString());
         }
 
         long startTime = timeoutNanos <= 0 ? 0 : System.nanoTime();
@@ -369,7 +370,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
     protected void checkDeadLock() {
         EventExecutor e = executor();
         if (e != null && e.inEventLoop()) {
-            throw new BlockingOperationException();
+            throw new BlockingOperationException(toString());
         }
     }
 
@@ -379,7 +380,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
             notifyListeners();
             return this;
         }
-        throw new IllegalStateException("complete already");
+        throw new IllegalStateException("complete already: " + this);
     }
 
     @Override
@@ -397,7 +398,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
             notifyListeners();
             return this;
         }
-        throw new IllegalStateException("complete already", cause);
+        throw new IllegalStateException("complete already: " + this, cause);
     }
 
     @Override
@@ -437,14 +438,14 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
     public boolean setUncancellable() {
         Object result = this.result;
         if (isDone0(result)) {
-            return isCancelled0(result);
+            return false;
         }
 
         synchronized (this) {
             // Allow only once.
             result = this.result;
             if (isDone0(result)) {
-                return isCancelled0(result);
+                return false;
             }
 
             this.result = UNCANCELLABLE;
@@ -509,7 +510,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
 
     private void incWaiters() {
         if (waiters == Short.MAX_VALUE) {
-            throw new IllegalStateException("too many waiters");
+            throw new IllegalStateException("too many waiters: " + this);
         }
         waiters ++;
     }
@@ -734,4 +735,30 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
             this.cause = cause;
         }
     }
+
+    @Override
+    public String toString() {
+        return toStringBuilder().toString();
+    }
+
+    protected StringBuilder toStringBuilder() {
+        StringBuilder buf = new StringBuilder(64);
+        buf.append(StringUtil.simpleClassName(this));
+        buf.append('@');
+        buf.append(Integer.toHexString(hashCode()));
+
+        Object result = this.result;
+        if (result == SUCCESS) {
+            buf.append("(success)");
+        } else if (result == UNCANCELLABLE) {
+            buf.append("(uncancellable)");
+        } else if (result instanceof CauseHolder) {
+            buf.append("(failure(");
+            buf.append(((CauseHolder) result).cause);
+            buf.append(')');
+        } else {
+            buf.append("(incomplete)");
+        }
+        return buf;
+    }
 }

common/src/main/java/io/netty/util/concurrent/PromiseTask.java

@@ -16,15 +16,41 @@
 package io.netty.util.concurrent;
 
 import java.util.concurrent.Callable;
-import java.util.concurrent.Executors;
 import java.util.concurrent.RunnableFuture;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 class PromiseTask<V> extends DefaultPromise<V> implements RunnableFuture<V> {
 
+    static <T> Callable<T> toCallable(Runnable runnable, T result) {
+        return new RunnableAdapter<T>(runnable, result);
+    }
+
+    private static final class RunnableAdapter<T> implements Callable<T> {
+        final Runnable task;
+        final T result;
+
+        RunnableAdapter(Runnable task, T result) {
+            this.task = task;
+            this.result = result;
+        }
+
+        @Override
+        public T call() {
+            task.run();
+            return result;
+        }
+
+        @Override
+        public String toString() {
+            return "Callable(task: " + task + ", result: " + result + ')';
+        }
+    }
+
     protected final Callable<V> task;
 
     PromiseTask(EventExecutor executor, Runnable runnable, V result) {
-        this(executor, Executors.callable(runnable, result));
+        this(executor, toCallable(runnable, result));
     }
 
     PromiseTask(EventExecutor executor, Callable<V> callable) {
@@ -45,8 +71,10 @@ class PromiseTask<V> extends DefaultPromise<V> implements RunnableFuture<V> {
     @Override
     public void run() {
         try {
+            if (setUncancellableInternal()) {
                 V result = task.call();
                 setSuccessInternal(result);
+            }
         } catch (Throwable e) {
             setFailureInternal(e);
         }
@@ -58,6 +86,8 @@ class PromiseTask<V> extends DefaultPromise<V> implements RunnableFuture<V> {
     }
 
     protected final Promise<V> setFailureInternal(Throwable cause) {
+        Logger.getLogger(PromiseTask.class.getName()).log(
+                Level.WARNING, "Calling setFailureInternal(" + cause + ") on " + this, new Exception());
         super.setFailure(cause);
         return this;
     }
@@ -77,6 +107,8 @@ class PromiseTask<V> extends DefaultPromise<V> implements RunnableFuture<V> {
     }
 
     protected final Promise<V> setSuccessInternal(V result) {
+        Logger.getLogger(PromiseTask.class.getName()).log(
+                Level.WARNING, "Calling setSuccessInternal() on " + this, new Exception());
         super.setSuccess(result);
         return this;
     }
@@ -98,4 +130,14 @@ class PromiseTask<V> extends DefaultPromise<V> implements RunnableFuture<V> {
     protected final boolean setUncancellableInternal() {
         return super.setUncancellable();
     }
+
+    @Override
+    protected StringBuilder toStringBuilder() {
+        StringBuilder buf = super.toStringBuilder();
+        buf.setCharAt(buf.length() - 1, ',');
+        buf.append(" task: ");
+        buf.append(task);
+        buf.append(')');
+        return buf;
+    }
 }

common/src/main/java/io/netty/util/concurrent/ScheduledFutureTask.java

@@ -19,7 +19,6 @@ package io.netty.util.concurrent;
 import java.util.Queue;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Delayed;
-import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -46,7 +45,7 @@ final class ScheduledFutureTask<V> extends PromiseTask<V> implements ScheduledFu
             EventExecutor executor, Queue<ScheduledFutureTask<?>> delayedTaskQueue,
             Runnable runnable, V result, long nanoTime) {
 
-        this(executor, delayedTaskQueue, Executors.callable(runnable, result), nanoTime);
+        this(executor, delayedTaskQueue, toCallable(runnable, result), nanoTime);
     }
 
     ScheduledFutureTask(
@@ -145,4 +144,18 @@ final class ScheduledFutureTask<V> extends PromiseTask<V> implements ScheduledFu
             setFailureInternal(cause);
         }
     }
+
+    @Override
+    protected StringBuilder toStringBuilder() {
+        StringBuilder buf = super.toStringBuilder();
+        buf.setCharAt(buf.length() - 1, ',');
+        buf.append(" id: ");
+        buf.append(id);
+        buf.append(", deadline: ");
+        buf.append(deadlineNanos);
+        buf.append(", period: ");
+        buf.append(periodNanos);
+        buf.append(')');
+        return buf;
+    }
 }