From f853b52a3b2dc97f750db0a3f6eeaf058fc8884a Mon Sep 17 00:00:00 2001 From: Eli Collins Date: Thu, 30 Jun 2011 17:54:20 +0000 Subject: [PATCH] HADOOP-7428. IPC connection is orphaned with null 'out' member. Contributed by Todd Lipcon git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1141638 13f79535-47bb-0310-9956-ffa450edef68 --- common/CHANGES.txt | 3 ++ .../java/org/apache/hadoop/ipc/Client.java | 8 ++- .../core/org/apache/hadoop/ipc/TestIPC.java | 51 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/common/CHANGES.txt b/common/CHANGES.txt index feae1ef333..4bdc0a720d 100644 --- a/common/CHANGES.txt +++ b/common/CHANGES.txt @@ -337,6 +337,9 @@ Trunk (unreleased changes) HADOOP-7402. TestConfiguration doesn't clean up after itself. (atm via eli) + HADOOP-7428. IPC connection is orphaned with null 'out' member. + (todd via eli) + Release 0.22.0 - Unreleased INCOMPATIBLE CHANGES diff --git a/common/src/java/org/apache/hadoop/ipc/Client.java b/common/src/java/org/apache/hadoop/ipc/Client.java index b4fb34531f..d1980f9c40 100644 --- a/common/src/java/org/apache/hadoop/ipc/Client.java +++ b/common/src/java/org/apache/hadoop/ipc/Client.java @@ -583,8 +583,12 @@ public Boolean run() throws IOException { start(); return; } - } catch (IOException e) { - markClosed(e); + } catch (Throwable t) { + if (t instanceof IOException) { + markClosed((IOException)t); + } else { + markClosed(new IOException("Couldn't set up IO streams", t)); + } close(); } } diff --git a/common/src/test/core/org/apache/hadoop/ipc/TestIPC.java b/common/src/test/core/org/apache/hadoop/ipc/TestIPC.java index 95156bfcdc..cf272026ed 100644 --- a/common/src/test/core/org/apache/hadoop/ipc/TestIPC.java +++ b/common/src/test/core/org/apache/hadoop/ipc/TestIPC.java @@ -23,6 +23,7 @@ import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.io.Writable; import org.apache.hadoop.io.LongWritable; +import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.net.NetUtils; @@ -45,6 +46,9 @@ import org.apache.hadoop.conf.Configuration; import org.junit.Assume; +import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import com.google.common.primitives.Bytes; import com.google.common.primitives.Ints; @@ -469,6 +473,53 @@ public void testSocketFactoryException() throws Exception { } } + /** + * Test that, if a RuntimeException is thrown after creating a socket + * but before successfully connecting to the IPC server, that the + * failure is handled properly. This is a regression test for + * HADOOP-7428. + */ + @Test + public void testRTEDuringConnectionSetup() throws Exception { + // Set up a socket factory which returns sockets which + // throw an RTE when setSoTimeout is called. + SocketFactory spyFactory = spy(NetUtils.getDefaultSocketFactory(conf)); + Mockito.doAnswer(new Answer() { + @Override + public Socket answer(InvocationOnMock invocation) throws Throwable { + Socket s = spy((Socket)invocation.callRealMethod()); + doThrow(new RuntimeException("Injected fault")).when(s) + .setSoTimeout(anyInt()); + return s; + } + }).when(spyFactory).createSocket(); + + Server server = new TestServer(1, true); + server.start(); + try { + // Call should fail due to injected exception. + InetSocketAddress address = NetUtils.getConnectAddress(server); + Client client = new Client(LongWritable.class, conf, spyFactory); + try { + client.call(new LongWritable(RANDOM.nextLong()), + address, null, null, 0, conf); + fail("Expected an exception to have been thrown"); + } catch (Exception e) { + LOG.info("caught expected exception", e); + assertTrue(StringUtils.stringifyException(e).contains( + "Injected fault")); + } + // Resetting to the normal socket behavior should succeed + // (i.e. it should not have cached a half-constructed connection) + + Mockito.reset(spyFactory); + client.call(new LongWritable(RANDOM.nextLong()), + address, null, null, 0, conf); + } finally { + server.stop(); + } + } + @Test public void testIpcTimeout() throws Exception { // start server