[HTTPCORE-207] SocketHttp*Connection classes can leak sockets if the connection is half-closed

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 4.0.1
• Fix Version/s: 4.1-beta1
• Component/s: HttpCore
• 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)
• Assignee: Unassigned
• Reporter: David Koski
• Created: 24/Sep/09 05:52
• Updated: 25/Apr/11 14:26
• Resolved: 24/Sep/09 19:08

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

}