core: log exceptions thrown by Runnable's executed via schedule(..). Fixes #1237

core/src/main/java/io/grpc/Deadline.java

@@ -34,6 +34,8 @@ package io.grpc;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import io.grpc.internal.LogExceptionRunnable;
+
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -148,7 +150,8 @@ public final class Deadline implements Comparable<Deadline> {
   public ScheduledFuture<?> runOnExpiration(Runnable task, ScheduledExecutorService scheduler) {
     Preconditions.checkNotNull(task, "task");
     Preconditions.checkNotNull(scheduler, "scheduler");
-    return scheduler.schedule(task, deadlineNanos - ticker.read(), TimeUnit.NANOSECONDS);
+    return scheduler.schedule(new LogExceptionRunnable(task),
+        deadlineNanos - ticker.read(), TimeUnit.NANOSECONDS);
   }
 
   @Override

core/src/main/java/io/grpc/DnsNameResolver.java

@@ -34,6 +34,7 @@ package io.grpc;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
+import io.grpc.internal.LogExceptionRunnable;
 import io.grpc.internal.SharedResourceHolder;
 import io.grpc.internal.SharedResourceHolder.Resource;
 
@@ -150,8 +151,9 @@ class DnsNameResolver extends NameResolver {
               }
               // Because timerService is the single-threaded GrpcUtil.TIMER_SERVICE in production,
               // we need to delegate the blocking work to the executor
-              resolutionTask = timerService.schedule(resolutionRunnableOnExecutor,
-                  1, TimeUnit.MINUTES);
+              resolutionTask =
+                  timerService.schedule(new LogExceptionRunnable(resolutionRunnableOnExecutor),
+                      1, TimeUnit.MINUTES);
             }
             savedListener.onError(Status.UNAVAILABLE.withCause(e));
             return;

core/src/main/java/io/grpc/internal/LogExceptionRunnable.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2016, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.internal;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * A simple wrapper for a {@link Runnable} that logs any exception thrown by it, before
+ * re-throwing it.
+ */
+public final class LogExceptionRunnable implements Runnable {
+
+  private static final Logger log = Logger.getLogger(LogExceptionRunnable.class.getName());
+
+  private final Runnable task;
+
+  public LogExceptionRunnable(Runnable task) {
+    this.task = checkNotNull(task);
+  }
+
+  @Override
+  public void run() {
+    try {
+      task.run();
+    } catch (Throwable t) {
+      log.log(Level.SEVERE, "Exception while executing runnable " + task, t);
+      throw t instanceof RuntimeException ? (RuntimeException) t : new RuntimeException(t);
+    }
+  }
+}

core/src/main/java/io/grpc/internal/SharedResourceHolder.java

@@ -149,8 +149,9 @@ public final class SharedResourceHolder {
       if (destroyer == null) {
         destroyer = destroyerFactory.createScheduledExecutor();
       }
-      cached.destroyTask = destroyer.schedule(new Runnable() {
-        @Override public void run() {
+      cached.destroyTask = destroyer.schedule(new LogExceptionRunnable(new Runnable() {
+        @Override
+        public void run() {
           synchronized (SharedResourceHolder.this) {
             // Refcount may have gone up since the task was scheduled. Re-check it.
             if (cached.refcount == 0) {
@@ -163,7 +164,7 @@ public final class SharedResourceHolder {
             }
           }
         }
-      }, DESTROY_DELAY_SECONDS, TimeUnit.SECONDS);
+      }), DESTROY_DELAY_SECONDS, TimeUnit.SECONDS);
     }
     // Always returning null
     return null;

core/src/main/java/io/grpc/internal/TransportSet.java

@@ -259,7 +259,7 @@ final class TransportSet implements WithLogId {
       }
     };
     reconnectTask = scheduledExecutor.schedule(
-        endOfCurrentBackoff, delayMillis, TimeUnit.MILLISECONDS);
+        new LogExceptionRunnable(endOfCurrentBackoff), delayMillis, TimeUnit.MILLISECONDS);
   }
 
   /**

interop-testing/src/main/java/io/grpc/testing/integration/TestServiceImpl.java

@@ -36,6 +36,7 @@ import com.google.protobuf.ByteString;
 import com.google.protobuf.EmptyProtos;
 
 import io.grpc.Status;
+import io.grpc.internal.LogExceptionRunnable;
 import io.grpc.stub.ServerCallStreamObserver;
 import io.grpc.stub.StreamObserver;
 import io.grpc.testing.integration.Messages.PayloadType;
@@ -336,7 +337,8 @@ public class TestServiceImpl implements TestServiceGrpc.TestService {
         Chunk nextChunk = chunks.peek();
         if (nextChunk != null) {
           scheduled = true;
-          executor.schedule(dispatchTask, nextChunk.delayMicroseconds, TimeUnit.MICROSECONDS);
+          executor.schedule(new LogExceptionRunnable(dispatchTask),
+              nextChunk.delayMicroseconds, TimeUnit.MICROSECONDS);
           return;
         }
       }