[JENKINS-45233] - Log errors when Response message cannot be delivere…

…d due to the closed channel (#177) * [FIXED JENKINS-45223] - Print warnings about failed responses when cannot deliver them due to the closed channel * Improve logging of the command execution * [JENKINS-45233] - Address comments from @jglick * [JENKINS-45233] - get rid of gratituos synchronization
jenkinsci · Sep 26, 2017 · b2c6ad1 · b2c6ad1
1 parent b4d3338
commit b2c6ad1
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java
@@ -528,11 +528,18 @@ protected Channel(@Nonnull ChannelBuilder settings, @Nonnull CommandTransport tr
         transport.setup(this, new CommandReceiver() {
             public void handle(Command cmd) {
                 commandsReceived++;
-                lastCommandReceivedAt = System.currentTimeMillis();
-                if (logger.isLoggable(Level.FINE))
+                long receivedAt = System.currentTimeMillis();
+                lastCommandReceivedAt = receivedAt;
+                if (logger.isLoggable(Level.FINE)) {
                     logger.fine("Received " + cmd);
+                } else if (logger.isLoggable(Level.FINER)) {
+                    logger.log(Level.FINER, "Received command " + cmd, cmd.createdAt);
+                }
                 try {
                     cmd.execute(Channel.this);
+                    if (logger.isLoggable(Level.FINE)) {
+                        logger.log(Level.FINE, "Completed command {0}. It took {1}ms", new Object[] {cmd, System.currentTimeMillis() - receivedAt});
+                    }
                 } catch (Throwable t) {
                     logger.log(Level.SEVERE, "Failed to execute command " + cmd + " (channel " + Channel.this.name + ")", t);
                     logger.log(Level.SEVERE, "This command is created here", cmd.createdAt);

diff --git a/src/main/java/hudson/remoting/Request.java b/src/main/java/hudson/remoting/Request.java
@@ -25,6 +25,7 @@
 
 import java.io.IOException;
 import java.io.Serializable;
+import java.nio.channels.ClosedChannelException;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
@@ -345,14 +346,11 @@ public void run() {
                     if(chainCause)
                         rsp.createdAt.initCause(createdAt);
 
-                    synchronized (channel) {// expand the synchronization block of the send() method to a check
-                        if(!channel.isOutClosed())
-                            channel.send(rsp);
-                    }
+                    channel.send(rsp);
                 } catch (IOException e) {
                     // communication error.
                     // this means the caller will block forever
-                    logger.log(Level.SEVERE, "Failed to send back a reply",e);
+                    logger.log(Level.WARNING, "Failed to send back a reply to the request " + this, e);
                 } finally {
                     channel.executingCalls.remove(id);
                     Thread.currentThread().setName(oldThreadName);