From 66bf21ba97df5c99605fe2e8c404fa6e47f1f571 Mon Sep 17 00:00:00 2001 From: kohsuke Date: Wed, 2 Sep 2009 23:06:15 +0000 Subject: [PATCH] Merged revisions 21385-21386 via svnmerge from https://svn.dev.java.net/svn/hudson/branches/rc ........ r21385 | kohsuke | 2009-09-02 15:41:39 -0700 (Wed, 02 Sep 2009) | 1 line Fixed a random test failure, which was caused by PingThread detecting communication problem while it's being orderly shut down. ........ r21386 | kohsuke | 2009-09-02 15:41:57 -0700 (Wed, 02 Sep 2009) | 1 line indentation fix. ........ git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@21387 71c3de6d-444a-0410-be80-ed276b4c234a --- remoting/pom.xml | 6 +++++ .../main/java/hudson/remoting/Channel.java | 2 +- .../remoting/ChannelClosedException.java | 14 +++++++++++ .../main/java/hudson/remoting/PingThread.java | 4 ++++ .../main/java/hudson/remoting/Request.java | 6 +++-- .../java/hudson/remoting/ChannelRunner.java | 23 +++++++++++++++---- .../java/hudson/remoting/RmiTestBase.java | 12 +++------- test/pom.xml | 2 +- 8 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 remoting/src/main/java/hudson/remoting/ChannelClosedException.java diff --git a/remoting/pom.xml b/remoting/pom.xml index 16aaef0307..07ebf83d0d 100644 --- a/remoting/pom.xml +++ b/remoting/pom.xml @@ -134,5 +134,11 @@ THE SOFTWARE. 1.3 test + + commons-io + commons-io + 1.4 + test + diff --git a/remoting/src/main/java/hudson/remoting/Channel.java b/remoting/src/main/java/hudson/remoting/Channel.java index 224468c541..d79db7d403 100644 --- a/remoting/src/main/java/hudson/remoting/Channel.java +++ b/remoting/src/main/java/hudson/remoting/Channel.java @@ -394,7 +394,7 @@ public class Channel implements VirtualChannel, IChannel { */ /*package*/ synchronized void send(Command cmd) throws IOException { if(outClosed) - throw new IOException("already closed"); + throw new ChannelClosedException(); if(logger.isLoggable(Level.FINE)) logger.fine("Send "+cmd); Channel old = Channel.setCurrent(this); diff --git a/remoting/src/main/java/hudson/remoting/ChannelClosedException.java b/remoting/src/main/java/hudson/remoting/ChannelClosedException.java new file mode 100644 index 0000000000..3bf42a1b6a --- /dev/null +++ b/remoting/src/main/java/hudson/remoting/ChannelClosedException.java @@ -0,0 +1,14 @@ +package hudson.remoting; + +import java.io.IOException; + +/** + * Indicates that the channel is already closed. + * + * @author Kohsuke Kawaguchi + */ +public class ChannelClosedException extends IOException { + public ChannelClosedException() { + super("channel is already closed"); + } +} diff --git a/remoting/src/main/java/hudson/remoting/PingThread.java b/remoting/src/main/java/hudson/remoting/PingThread.java index 49575b8747..09cee3fa27 100644 --- a/remoting/src/main/java/hudson/remoting/PingThread.java +++ b/remoting/src/main/java/hudson/remoting/PingThread.java @@ -74,6 +74,8 @@ public abstract class PingThread extends Thread { while((diff=nextCheck-System.currentTimeMillis())>0) Thread.sleep(diff); } + } catch (ChannelClosedException e) { + LOGGER.fine(getName()+" is closed. Terminating"); } catch (IOException e) { onDead(); } catch (InterruptedException e) { @@ -87,6 +89,8 @@ public abstract class PingThread extends Thread { try { f.get(TIME_OUT,MILLISECONDS); } catch (ExecutionException e) { + if (e.getCause() instanceof RequestAbortedException) + return; // connection has shut down orderly. onDead(); } catch (TimeoutException e) { onDead(); diff --git a/remoting/src/main/java/hudson/remoting/Request.java b/remoting/src/main/java/hudson/remoting/Request.java index d1788c662d..115b2aa1e1 100644 --- a/remoting/src/main/java/hudson/remoting/Request.java +++ b/remoting/src/main/java/hudson/remoting/Request.java @@ -243,8 +243,10 @@ abstract class Request extends C if(chainCause) rsp.createdAt.initCause(createdAt); - if(!channel.isOutClosed()) - channel.send(rsp); + synchronized (channel) {// expand the synchronization block of the send() method to a check + if(!channel.isOutClosed()) + channel.send(rsp); + } } catch (IOException e) { // communication error. // this means the caller will block forever diff --git a/remoting/src/test/java/hudson/remoting/ChannelRunner.java b/remoting/src/test/java/hudson/remoting/ChannelRunner.java index b35fad1ea4..da1cca24b7 100644 --- a/remoting/src/test/java/hudson/remoting/ChannelRunner.java +++ b/remoting/src/test/java/hudson/remoting/ChannelRunner.java @@ -29,11 +29,15 @@ import java.io.IOException; import java.io.PipedInputStream; import java.io.PipedOutputStream; import java.io.File; +import java.io.OutputStream; +import java.io.FileOutputStream; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.net.URLClassLoader; import java.net.URL; +import org.apache.commons.io.output.TeeOutputStream; + /** * Hides the logic of starting/stopping a channel for test. * @@ -120,24 +124,30 @@ interface ChannelRunner { proc = Runtime.getRuntime().exec("java -cp "+getClasspath()+" hudson.remoting.Launcher"); - copier = new Copier("copier",proc.getErrorStream(),System.err); + copier = new Copier("copier",proc.getErrorStream(),System.out); copier.start(); executor = Executors.newCachedThreadPool(); - return new Channel("north", executor, proc.getInputStream(), proc.getOutputStream()); + OutputStream out = proc.getOutputStream(); + if (RECORD_OUTPUT) { + File f = File.createTempFile("remoting",".log"); + System.out.println("Recording to "+f); + out = new TeeOutputStream(out,new FileOutputStream(f)); + } + return new Channel("north", executor, proc.getInputStream(), out); } public void stop(Channel channel) throws Exception { channel.close(); channel.join(10*1000); - System.out.println("north completed"); +// System.out.println("north completed"); executor.shutdown(); copier.join(); int r = proc.waitFor(); - System.out.println("south completed"); +// System.out.println("south completed"); Assert.assertEquals("exit code should have been 0",0,r); } @@ -156,5 +166,10 @@ interface ChannelRunner { } return buf.toString(); } + + /** + * Record the communication to the remote node. Used during debugging. + */ + private static boolean RECORD_OUTPUT = false; } } diff --git a/remoting/src/test/java/hudson/remoting/RmiTestBase.java b/remoting/src/test/java/hudson/remoting/RmiTestBase.java index 15aef00a0f..dc30d611c7 100644 --- a/remoting/src/test/java/hudson/remoting/RmiTestBase.java +++ b/remoting/src/test/java/hudson/remoting/RmiTestBase.java @@ -23,18 +23,11 @@ */ package hudson.remoting; -import junit.framework.TestCase; +import hudson.remoting.ChannelRunner.InProcess; import junit.framework.Test; +import junit.framework.TestCase; import junit.framework.TestSuite; -import java.io.IOException; -import java.io.PipedInputStream; -import java.io.PipedOutputStream; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; - -import hudson.remoting.ChannelRunner.InProcess; - /** * Base class for remoting tests. * @@ -46,6 +39,7 @@ public abstract class RmiTestBase extends TestCase { private ChannelRunner channelRunner = new InProcess(); protected void setUp() throws Exception { + System.out.println("Starting "+getName()); channel = channelRunner.start(); } diff --git a/test/pom.xml b/test/pom.xml index 65cd73a6ac..3f75176991 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -60,7 +60,7 @@ THE SOFTWARE. org.codehaus.groovy.maven gmaven-plugin - 1.0-rc-5 + 1.0-rc-5 preset-packager -- GitLab