Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r947747 - in /hadoop/common/trunk: CHANGES.txt
 src/java/org/apache/hadoop/ipc/Client.java
 src/test/core/org/apache/hadoop/ipc/TestIPC.java
Date: Mon, 24 May 2010 18:24:16 -0000
To: common-commits@hadoop.apache.org
From: tomwhite@apache.org
Message-Id: <20100524182417.182BB23888E8@eris.apache.org>

Author: tomwhite
Date: Mon May 24 18:24:16 2010
New Revision: 947747

URL: http://svn.apache.org/viewvc?rev=947747&view=rev
Log:
HADOOP-6723.  Unchecked exceptions thrown in IPC Connection should not orphan clients.  Contributed by Todd Lipcon.

Modified:
    hadoop/common/trunk/CHANGES.txt
    hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java
    hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java

Modified: hadoop/common/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/CHANGES.txt?rev=947747&r1=947746&r2=947747&view=diff
==============================================================================
--- hadoop/common/trunk/CHANGES.txt (original)
+++ hadoop/common/trunk/CHANGES.txt Mon May 24 18:24:16 2010
@@ -1569,6 +1569,9 @@ Release 0.21.0 - Unreleased
     despite failure at any level. (Contributed by Ravi Gummadi and 
     Vinod Kumar Vavilapalli)
 
+    HADOOP-6723.  Unchecked exceptions thrown in IPC Connection should not
+    orphan clients.  (Todd Lipcon via tomwhite)
+
 Release 0.20.3 - Unreleased
 
   NEW FEATURES

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java?rev=947747&r1=947746&r2=947747&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/ipc/Client.java Mon May 24 18:24:16 2010
@@ -602,8 +602,16 @@ public class Client {
         LOG.debug(getName() + ": starting, having connections " 
             + connections.size());
 
-      while (waitForWork()) {//wait here for work - read or close connection
-        receiveResponse();
+      try {
+        while (waitForWork()) {//wait here for work - read or close connection
+          receiveResponse();
+        }
+      } catch (Throwable t) {
+        // This truly is unexpected, since we catch IOException in receiveResponse
+        // -- this is only to be really sure that we don't leave a client hanging
+        // forever.
+        LOG.warn("Unexpected error reading responses on connection " + this, t);
+        markClosed(new IOException("Error reading responses", t));
       }
       
       close();

Modified: hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java?rev=947747&r1=947746&r2=947747&view=diff
==============================================================================
--- hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java (original)
+++ hadoop/common/trunk/src/test/core/org/apache/hadoop/ipc/TestIPC.java Mon May 24 18:24:16 2010
@@ -249,6 +249,23 @@ public class TestIPC extends TestCase {
       throw new IOException(ERR_MSG);
     }
   }
+  
+  private static class LongRTEWritable extends LongWritable {
+    private final static String ERR_MSG = 
+      "Come across an runtime exception while reading";
+    
+    LongRTEWritable() {}
+    
+    LongRTEWritable(long longValue) {
+      super(longValue);
+    }
+    
+    public void readFields(DataInput in) throws IOException {
+      super.readFields(in);
+      throw new RuntimeException(ERR_MSG);
+    }
+  }
+
   public void testErrorClient() throws Exception {
     // start server
     Server server = new TestServer(1, false);
@@ -268,6 +285,30 @@ public class TestIPC extends TestCase {
       assertEquals(LongErrorWritable.ERR_MSG, cause.getMessage());
     }
   }
+  
+  public void testRuntimeExceptionWritable() throws Exception {
+    // start server
+    Server server = new TestServer(1, false);
+    InetSocketAddress addr = NetUtils.getConnectAddress(server);
+    server.start();
+
+    // start client
+    Client client = new Client(LongRTEWritable.class, conf);
+    try {
+      client.call(new LongRTEWritable(RANDOM.nextLong()),
+              addr, null, null);
+      fail("Expected an exception to have been thrown");
+    } catch (IOException e) {
+      // check error
+      Throwable cause = e.getCause();
+      assertTrue(cause instanceof IOException);
+      // it's double-wrapped
+      Throwable cause2 = cause.getCause();
+      assertTrue(cause2 instanceof RuntimeException);
+
+      assertEquals(LongRTEWritable.ERR_MSG, cause2.getMessage());
+    }
+  }
 
   /**
    * Test that, if the socket factory throws an IOE, it properly propagates