[jira] Created: (HTTPCLIENT-879) DefaultClientConnection can leaked CLOSED connection

DefaultClientConnection can leaked CLOSED connection
----------------------------------------------------

                 Key: HTTPCLIENT-879
                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-879
             Project: HttpComponents HttpClient
          Issue Type: Bug
          Components: HttpClient
    Affects Versions: 4.0.1
         Environment: 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)
            Reporter: David Koski


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);
    }
}


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCORE-207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski moved HTTPCLIENT-879 to HTTPCORE-207:
-------------------------------------------------------

          Component/s:     (was: HttpClient)
                       HttpCore
    Affects Version/s:     (was: 4.0.1)
                       4.0.1
                  Key: HTTPCORE-207  (was: HTTPCLIENT-879)
              Project: HttpComponents HttpCore  (was: HttpComponents HttpClient)

> DefaultClientConnection can leaked CLOSED connection
> ----------------------------------------------------
>
>                 Key: HTTPCORE-207
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-207
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore
>    Affects Versions: 4.0.1
>         Environment: 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)
>            Reporter: David Koski
>
> 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);
>     }
> }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCORE-207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated HTTPCORE-207:
---------------------------------------

    Attachment: HTTPCORE-207.patch

Good catch, David. Please review / test the patch.

Oleg

> SocketHttp*Connection classes can leak sockets if the connection is half-closed
> -------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-207
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-207
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore
>    Affects Versions: 4.0.1
>         Environment: 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)
>            Reporter: David Koski
>         Attachments: HTTPCORE-207.patch
>
>
> 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);
>     }
> }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCORE-207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated HTTPCORE-207:
---------------------------------------

    Summary: SocketHttp*Connection classes can leak sockets if the connection is half-closed  (was: DefaultClientConnection can leaked CLOSED connection)

> SocketHttp*Connection classes can leak sockets if the connection is half-closed
> -------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-207
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-207
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore
>    Affects Versions: 4.0.1
>         Environment: 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)
>            Reporter: David Koski
>         Attachments: HTTPCORE-207.patch
>
>
> 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);
>     }
> }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

    [ https://issues.apache.org/jira/browse/HTTPCORE-207?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12759140#action_12759140 ]

David Koski commented on HTTPCORE-207:
--------------------------------------

sock.close() moves into a finally block which looks good to me.

> SocketHttp*Connection classes can leak sockets if the connection is half-closed
> -------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-207
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-207
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore
>    Affects Versions: 4.0.1
>         Environment: 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)
>            Reporter: David Koski
>         Attachments: HTTPCORE-207.patch
>
>
> 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);
>     }
> }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...

     [ https://issues.apache.org/jira/browse/HTTPCORE-207?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCORE-207.
----------------------------------------

       Resolution: Fixed
    Fix Version/s: 4.1-beta1
                   4.0.2

Committed to trunk and 4.0.x branch.

Oleg

> SocketHttp*Connection classes can leak sockets if the connection is half-closed
> -------------------------------------------------------------------------------
>
>                 Key: HTTPCORE-207
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-207
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore
>    Affects Versions: 4.0.1
>         Environment: 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)
>            Reporter: David Koski
>             Fix For: 4.0.2, 4.1-beta1
>
>         Attachments: HTTPCORE-207.patch
>
>
> 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);
>     }
> }

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@...
For additional commands, e-mail: dev-help@...