[FIXED JENKINS-7114] fixed the layer confusion in the remoting

When ProxyOutputStream sends write(byte[]) to the other end and the actual write fails, "NotifyDeadWriter" object comes back and reports back that the write has failed. Without this mechanism, the writer side will keep on going. Because the RemoteOutputStream service is a lower layer service that cannot rely on custom classloading service (doing so would create cyclic dependencies), when we send back this exception, the object graph of the exception needs to be deserializable on the receiver side. This is not the case when the exception class is defined by the user (as is the case of winstone.ClientSocketException.) This fix addresses this problem by turning an exception class into another class that emulates the output of the original exception. To make this change interopeable with earlier versions, we need to introduce a new capability flag. If we send MimicException to the other side and the other side doesn't have this class definition, then it'll cause automatic fail.
jenkinsci · Dec 20, 2012 · d084f50 · d084f50
1 parent ff61d35
commit d084f50
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 10 deletions.
diff --git a/src/main/java/hudson/remoting/Capability.java b/src/main/java/hudson/remoting/Capability.java
@@ -33,7 +33,7 @@ public final class Capability implements Serializable {
     }
 
     public Capability() {
-        this(MASK_MULTI_CLASSLOADER|MASK_PIPE_THROTTLING);
+        this(MASK_MULTI_CLASSLOADER|MASK_PIPE_THROTTLING|MASK_MIMIC_EXCEPTION);
     }
 
     /**
@@ -55,6 +55,10 @@ public boolean supportsPipeThrottling() {
         return (mask& MASK_PIPE_THROTTLING)!=0;
     }
 
+    public boolean hasMimicException() {
+        return (mask&MASK_MIMIC_EXCEPTION)!=0;
+    }
+
     /**
      * Writes out the capacity preamble.
      */
@@ -103,6 +107,11 @@ public static Capability read(InputStream is) throws IOException {
      */
     private static final long MASK_PIPE_THROTTLING = 4L;
 
+    /**
+     * Supports {@link MimicException}.
+     */
+    private static final long MASK_MIMIC_EXCEPTION = 8L;
+
     static final byte[] PREAMBLE;
 
     public static final Capability NONE = new Capability(0);

diff --git a/src/main/java/hudson/remoting/Channel.java b/src/main/java/hudson/remoting/Channel.java
@@ -840,8 +840,8 @@ public synchronized void join(long timeout) throws InterruptedException {
      * {@link ObjectOutputStream}, and it's the last command to be read.
      */
     private static final class CloseCommand extends Command {
-        private CloseCommand(Throwable cause) {
-            super(cause);
+        private CloseCommand(Channel channel, Throwable cause) {
+            super(channel,cause);
         }
 
         protected void execute(Channel channel) {
@@ -906,7 +906,7 @@ public synchronized void close() throws IOException {
     public synchronized void close(Throwable diagnosis) throws IOException {
         if(outClosed!=null)  return;  // already closed
 
-        send(new CloseCommand(diagnosis));
+        send(new CloseCommand(this,diagnosis));
         outClosed = new IOException().initCause(diagnosis);   // last command sent. no further command allowed. lock guarantees that no command will slip inbetween
         try {
             transport.closeWrite();

diff --git a/src/main/java/hudson/remoting/Command.java b/src/main/java/hudson/remoting/Command.java
@@ -50,8 +50,10 @@ protected Command() {
         this(true);
     }
 
-    protected Command(Throwable cause) {
-        this.createdAt = new Source(cause);
+    protected Command(Channel channel, Throwable cause) {
+        // Command object needs to be deserializable on the other end without requiring custom classloading,
+        // so we wrap this in MimicException
+        this.createdAt = new Source(MimicException.make(channel,cause));
     }
 
     /**

diff --git a/src/main/java/hudson/remoting/MimicException.java b/src/main/java/hudson/remoting/MimicException.java
@@ -0,0 +1,41 @@
+package hudson.remoting;
+
+/**
+ * Exception that prints like the specified exception.
+ *
+ * This is used to carry the diagnostic information to the other side of the channel
+ * in situations where we cannot use class remoting.
+ *
+ * @author Kohsuke Kawaguchi
+ * @see Capability#hasMimicException()
+ */
+class MimicException extends Exception {
+    private final String className;
+    MimicException(Throwable cause) {
+        super(cause.getMessage());
+        className = cause.getClass().getName();
+        setStackTrace(cause.getStackTrace());
+
+        if (cause.getCause()!=null)
+            initCause(new MimicException(cause.getCause()));
+    }
+
+    @Override
+    public String toString() {
+        String s = className;
+        String message = getLocalizedMessage();
+        return (message != null) ? (s + ": " + message) : s;
+    }
+
+    public static Throwable make(Channel ch, Throwable cause) {
+        if (cause == null)  return null;
+
+        // make sure the remoting layer of the other end supports this
+        if (ch.remoteCapability.hasMimicException())
+            return new MimicException(cause);
+        else
+            return cause;
+    }
+
+    private static final long serialVersionUID = 1L;
+}
diff --git a/src/main/java/hudson/remoting/ProxyOutputStream.java b/src/main/java/hudson/remoting/ProxyOutputStream.java
@@ -216,7 +216,7 @@ public void run() {
                         os.write(buf);
                     } catch (IOException e) {
                         try {
-                            channel.send(new NotifyDeadWriter(e,oid));
+                            channel.send(new NotifyDeadWriter(channel,e,oid));
                         } catch (ChannelClosedException x) {
                             // the other direction can be already closed if the connection
                             // shut down is initiated from this side. In that case, remain silent.
@@ -385,8 +385,8 @@ public String toString() {
     private static final class NotifyDeadWriter extends Command {
         private final int oid;
 
-        private NotifyDeadWriter(Throwable cause, int oid) {
-            super(cause);
+        private NotifyDeadWriter(Channel channel,Throwable cause, int oid) {
+            super(channel,cause);
             this.oid = oid;
         }
 

diff --git a/src/test/java/hudson/remoting/DeadRemoteOutputStreamTest.java b/src/test/java/hudson/remoting/DeadRemoteOutputStreamTest.java
@@ -2,7 +2,9 @@
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.io.PrintWriter;
 import java.io.Serializable;
+import java.io.StringWriter;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -37,7 +39,10 @@ public Void call() throws Exception {
                     }
                     fail("Expected to see the failure");
                 } catch (IOException e) {
-                    assertTrue(e.getMessage().contains(MESSAGE));
+                    StringWriter sw = new StringWriter();
+                    e.printStackTrace(new PrintWriter(sw));
+                    String whole = sw.toString();
+                    assertTrue(whole, whole.contains(MESSAGE) && whole.contains("hudson.rem0ting.TestCallable"));
                 }
                 return null;
             }