Uploaded image for project: 'HttpComponents HttpCore'
  1. HttpComponents HttpCore
  2. HTTPCORE-207

SocketHttp*Connection classes can leak sockets if the connection is half-closed

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 4.0.1
    • 4.1-beta1
    • HttpCore
    • None
    • Mac OS X 10.6.1

      gorgatron% java -version
      java version "1.6.0_15"
      Java(TM) SE Runtime Environment (build 1.6.0_15-b03-219)
      Java HotSpot(TM) 64-Bit Server VM (build 14.1-b02-90, mixed mode)

    Description

      Unit test below. Basically what happens is this:

      • turn off stale check
      • GET request to X
      • wait for the connection to go to CLOSE_WAIT from idle
      • wait for the connection to go to CLOSED
      • GET request to X
      • tries that request
      • throws IOException
      • calls connection.close()
      • close() sets the open = false
      • calls doFlush()
      • that throws
      • caught and close is called again
      • but open = false, so it returns – the Socket object is never closed

      Unit test below with proposed fix (hack in style, but you get the picture). I use a connection:close to make it run in less than 10 minutes

      After it gets to:

      System.out.println("second call done");

      You can do something like this:

      gorgatron% lsof | grep java | grep TCP

      If you see two connections to www.apple.com:80, one CLOSED, that is the bug.

      import java.io.IOException;
      import java.lang.reflect.Field;
      import java.net.URI;
      import java.util.Arrays;

      import org.apache.http.HttpResponse;
      import org.apache.http.client.ClientProtocolException;
      import org.apache.http.client.ResponseHandler;
      import org.apache.http.client.methods.HttpGet;
      import org.apache.http.client.protocol.RequestAddCookies;
      import org.apache.http.client.protocol.RequestClientConnControl;
      import org.apache.http.client.protocol.RequestDefaultHeaders;
      import org.apache.http.client.protocol.RequestProxyAuthentication;
      import org.apache.http.client.protocol.RequestTargetAuthentication;
      import org.apache.http.client.protocol.ResponseProcessCookies;
      import org.apache.http.conn.ClientConnectionOperator;
      import org.apache.http.conn.OperatedClientConnection;
      import org.apache.http.conn.scheme.PlainSocketFactory;
      import org.apache.http.conn.scheme.Scheme;
      import org.apache.http.conn.scheme.SchemeRegistry;
      import org.apache.http.impl.SocketHttpClientConnection;
      import org.apache.http.impl.client.DefaultHttpClient;
      import org.apache.http.impl.conn.DefaultClientConnection;
      import org.apache.http.impl.conn.DefaultClientConnectionOperator;
      import org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager;
      import org.apache.http.params.BasicHttpParams;
      import org.apache.http.params.HttpConnectionParams;
      import org.apache.http.params.HttpParams;
      import org.apache.http.protocol.BasicHttpProcessor;
      import org.apache.http.protocol.RequestContent;
      import org.apache.http.protocol.RequestExpectContinue;
      import org.apache.http.protocol.RequestTargetHost;
      import org.apache.http.protocol.RequestUserAgent;
      import org.apache.log4j.Level;
      import org.apache.log4j.Logger;

      public class LeakTest extends TestCase {

      public void testLeak() throws Exception {

      Logger.getLogger("org.apache.http.impl.client.DefaultRequestDirector").setLevel(Level.DEBUG);

      /*

      • Trigger #1: turn off stale check.
        */
        final HttpParams params = new BasicHttpParams();
        HttpConnectionParams.setStaleCheckingEnabled(params, false);

      final SchemeRegistry schemeRegistry = new SchemeRegistry();
      schemeRegistry.register(new Scheme("http", PlainSocketFactory.getSocketFactory(), 80));

      final Field f = SocketHttpClientConnection.class.getDeclaredField("open");
      f.setAccessible(true);

      final ThreadSafeClientConnManager connectionManager = new ThreadSafeClientConnManager(params, schemeRegistry) {

      @Override
      protected ClientConnectionOperator createConnectionOperator(final SchemeRegistry schreg) {
      return new DefaultClientConnectionOperator(schreg) {

      @Override
      public OperatedClientConnection createConnection() {
      return new DefaultClientConnection() {

      @Override
      public void close() throws IOException {
      if (true)

      { // this case will fail super.close(); }

      else {
      // this is the proposed fix
      if (!isOpen())

      { return; }

      try

      { f.set(this, false); }

      catch (final Exception e)

      { // eat it }

      try {
      doFlush();
      try {
      try

      { getSocket().shutdownOutput(); }

      catch (final IOException ignore) {
      }
      try

      { getSocket().shutdownInput(); }

      catch (final IOException ignore) {
      }
      } catch (final UnsupportedOperationException ignore)

      { // if one isn't supported, the other one isn't either }

      } finally

      { getSocket().close(); }

      }
      }
      };
      }
      };
      }

      };

      final DefaultHttpClient client = new DefaultHttpClient(connectionManager, params);

      final HttpGet method = new HttpGet(new URI("http://www.apple.com"));

      /*

      • Trigger #2: tell it connection: close. This is talking to a netscaler and it responds
      • with:
      • connection: keep-alive
      • but it actually closes the connection. I am not sure this is very important – if we do
      • not have this we would have to wait for the connection to go idle and get closed on the
      • remote side.
      • This command shows an example of this behavior:
      • curl -v -k -H connection:close "http://www.apple.com" "http://www.apple.com"
        */
        method.setHeader("connection", "close");

      client.execute(method, new ResponseHandler<Object>() {

      public Object handleResponse(final HttpResponse response) throws ClientProtocolException, IOException

      { System.out.println("First request headers = " + Arrays.toString(response.getAllHeaders())); return null; }

      });

      System.out.println("waiting...");

      /*

      • At this point the connection is in CLOSE_WAIT:
      • java 15681 dkoski 67u IPv6 0x117a1e20 0t0 TCP [::x.x.x.x]:53935->[::x.x.x.x]:8501
      • (CLOSE_WAIT)
      • Now wait for it to go into CLOSED.
        */

      waitForMilliseconds(1000 * 65);

      /*

      • Now that connection is CLOSED:
      • java 15681 dkoski 67u IPv6 0x117a1e20 0t0 TCP [::x.x.x.x]:53935->[::x.x.x.x]:8501
      • (CLOSED)
      • Execute the next request.
        */

      client.execute(method, new ResponseHandler<Object>() {

      public Object handleResponse(final HttpResponse response) throws ClientProtocolException, IOException

      { System.out.println("Second request headers = " + Arrays.toString(response.getAllHeaders())); return null; }

      });

      /**

      • And we seem to have leaked the first connection (at this point perhaps not much more than
      • an fd) and the second connection is going into CLOSE_WAIT:
      • java 15681 dkoski 67u IPv6 0x117a1e20 0t0 TCP [::x.x.x.x]:53935->[::x.x.x.x]:8501
      • (CLOSED)
      • java 15681 dkoski 69u IPv6 0x117a11f0 0t0 TCP [::x.x.x.x]:53936->[::x.x.x.x]:8501
      • (CLOSE_WAIT)
      • What happens inside DefaultRequestDirector:
      • throws an IOException (broken pipe) – right
      • org.apache.http.impl.conn.tsccm.BasicPooledConnAdapter@3414a97b -> close()
      • org.apache.http.impl.conn.DefaultClientConnection@70d9cbcb -> close()
      • open = false
      • SocketHttpClientConnection has:
      • if (!this.open) { return;
      • however, socket.closed = false (and truly it has not been closed)
      • The reason it is not closed is:
      • <pre>
      • java.net.SocketException: Broken pipe
      • at java.net.SocketOutputStream.socketWrite0(Native Method)
      • at java.net.SocketOutputStream.socketWrite(SocketOutputStream.java:92)
      • at java.net.SocketOutputStream.write(SocketOutputStream.java:136)
      • at org.apache.http.impl.io.AbstractSessionOutputBuffer.flushBuffer(AbstractSessionOutputBuffer.java:106)
      • at org.apache.http.impl.io.AbstractSessionOutputBuffer.flush(AbstractSessionOutputBuffer.java:113)
      • at org.apache.http.impl.AbstractHttpClientConnection.doFlush(AbstractHttpClientConnection.java:260)
      • at org.apache.http.impl.AbstractHttpClientConnection.flush(AbstractHttpClientConnection.java:265)
      • at org.apache.http.impl.conn.AbstractClientConnAdapter.flush(AbstractClientConnAdapter.java:197)
      • at org.apache.http.protocol.HttpRequestExecutor.doSendRequest(HttpRequestExecutor.java:252)
      • at org.apache.http.protocol.HttpRequestExecutor.execute(HttpRequestExecutor.java:124)
      • at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:447)
      • at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:641)
      • at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:730)
      • at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:708)
      • at org.apache.http.impl.client.AbstractHttpClient.execute(AbstractHttpClient.java:699)
      • </pre>
      • the code at fault seems to be:
      • <pre>
      • public void close() throws IOException {
      • if (!this.open) { * return; * }
      • this.open = false;
      • doFlush(); HERE: throws here and leaves the open = false but doesn't actually close it
      • try {
      • try { * this.socket.shutdownOutput(); * }

        catch (IOException ignore)

        { * }
        * try { * this.socket.shutdownInput(); * } catch (IOException ignore) { * }
      • } catch (UnsupportedOperationException ignore) { * // if one isn't supported, the other one isn't either * }
      • this.socket.close();
      • }
      • </pre>
        */

      System.out.println("second call done");
      }

      public void testWait()

      { waitForMilliseconds(1000 * 1000); }

      }

      Attachments

        1. HTTPCORE-207.patch
          3 kB
          Oleg Kalnichevski

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            dkoski David Koski
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Issue deployment