Merge pull request #3861 from agherardi/properlyClose

Properly close the Apache response so that connections can be reused

Author: senivam

connectors/apache-connector/src/main/java/org/glassfish/jersey/apache/connector/ApacheConnector.java

@@ -462,7 +462,7 @@ public ClientResponse apply(final ClientRequest clientRequest) throws Processing
             }
 
             try {
-                responseContext.setEntityStream(new HttpClientResponseInputStream(getInputStream(response)));
+                responseContext.setEntityStream(getInputStream(response));
             } catch (final IOException e) {
                 LOGGER.log(Level.SEVERE, null, e);
             }
@@ -601,18 +601,6 @@ private static Map<String, String> writeOutBoundHeaders(final MultivaluedMap<Str
         return stringHeaders;
     }
 
-    private static final class HttpClientResponseInputStream extends FilterInputStream {
-
-        HttpClientResponseInputStream(final InputStream inputStream) throws IOException {
-            super(inputStream);
-        }
-
-        @Override
-        public void close() throws IOException {
-            super.close();
-        }
-    }
-
     private static InputStream getInputStream(final CloseableHttpResponse response) throws IOException {
 
         final InputStream inputStream;
@@ -631,8 +619,13 @@ private static InputStream getInputStream(final CloseableHttpResponse response)
         return new FilterInputStream(inputStream) {
             @Override
             public void close() throws IOException {
-                response.close();
-                super.close();
+                try {
+                    super.close();
+                } catch (IOException ex) {
+                    // Ignore
+                } finally {
+                    response.close();
+                }
             }
         };
     }

connectors/apache-connector/src/test/java/org/glassfish/jersey/apache/connector/StreamingTest.java

@@ -25,10 +25,13 @@
 import javax.ws.rs.client.WebTarget;
 import javax.ws.rs.core.Application;
 import javax.ws.rs.core.MediaType;
-
+import javax.ws.rs.core.Response;
 import javax.inject.Singleton;
 
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+
 import org.glassfish.jersey.client.ClientConfig;
+import org.glassfish.jersey.client.ClientProperties;
 import org.glassfish.jersey.server.ChunkedOutput;
 import org.glassfish.jersey.server.ResourceConfig;
 import org.glassfish.jersey.test.JerseyTest;
@@ -40,14 +43,16 @@
  * @author Petr Janouch (petr.janouch at oracle.com)
  */
 public class StreamingTest extends JerseyTest {
+    private PoolingHttpClientConnectionManager connectionManager;
 
     /**
      * Test that a data stream can be terminated from the client side.
      */
     @Test
     public void clientCloseTest() throws IOException {
         // start streaming
-        InputStream inputStream = target().path("/streamingEndpoint").request().get(InputStream.class);
+        InputStream inputStream = target().path("/streamingEndpoint").request()
+                .property(ClientProperties.READ_TIMEOUT, 1_000).get(InputStream.class);
 
         WebTarget sendTarget = target().path("/streamingEndpoint/send");
         // trigger sending 'A' to the stream; OK is sent if everything on the server was OK
@@ -61,8 +66,35 @@ public void clientCloseTest() throws IOException {
         assertEquals("NOK", sendTarget.request().get().readEntity(String.class));
     }
 
+    /**
+     * Tests that closing a response after completely reading the entity reuses the connection
+     */
+    @Test
+    public void reuseConnectionTest() throws IOException {
+        Response response = target().path("/streamingEndpoint/get").request().get();
+        InputStream is = response.readEntity(InputStream.class);
+        byte[] buf = new byte[8192];
+        is.read(buf);
+        is.close();
+        response.close();
+
+        assertEquals(1, connectionManager.getTotalStats().getAvailable());
+        assertEquals(0, connectionManager.getTotalStats().getLeased());
+    }
+
+    /**
+     * Tests that closing a request without reading the entity does not throw an exception.
+     */
+    @Test
+    public void clientCloseThrowsNoExceptionTest() throws IOException {
+        Response response = target().path("/streamingEndpoint/get").request().get();
+        response.close();
+    }
+
     @Override
     protected void configureClient(ClientConfig config) {
+        connectionManager = new PoolingHttpClientConnectionManager();
+        config.property(ApacheClientProperties.CONNECTION_MANAGER, connectionManager);
         config.connectorProvider(new ApacheConnectorProvider());
     }
 
@@ -94,5 +126,12 @@ public String sendEvent() {
         public ChunkedOutput<String> get() {
             return output;
         }
+
+        @GET
+        @Path("get")
+        @Produces(MediaType.TEXT_PLAIN)
+        public String getString() {
+            return "OK";
+        }
     }
 }