Community
Participate
Working Groups
The update manager leaks sockets if requested URLs receive HTTP-404 responses. Excessive socket leakage may effectively shutdown the network interface in Windows 2000 until Eclipse is terminated. The socket leakage was verified with appropriate tools (SysInternal's TCPView) This effect is 100% reproducible. A prerequisite for recreation is an update site that returns numerous HTTP-404 in download requests. My local 3.2rc5 configuration fails when updating from the Callisto update site. Observation: 1 - verification of the URL request's response status org.eclipse.update.internal.core.UpdateManagerUtils.checkConnectionResult() #554 +-> HttpResponse.getStatusCode() #189 +-> HttpURLConnection.getResponseCode() +-> URLConnection.getInputStream() Please note that URLConnection.getResponseCode() implicitly opens the internal input stream. For HTTP_OK responses the code path later gets the opened input stream instance and properly closes it. 2 - However, if status != HTTP_OK, a FatalIOException is thrown in UpdateManagerUtils.checkConnectionResult(), this will leave the input stream open and leaks a socket until the Update Manager / JVM is closed. Proposed solution: before throwing the FatalIOException in checkConnectionResult(), ensure any implicitly opened input stream in the response object is closed. e. g. "response.getInputStream().close()" Currently, there seems not to be an explicit way to close a IResponse object, like: public void HttpResponse.close() { if( null != in ) { in.close(); in = null; } }
Can someone from the Update team please take a look during M7 performance pass.
The suggested fix implies reliance on the internal implementation. There is nothing in the java.net.HTTPUrlConnection that implies that 'getStatusCode' creates the input stream that needs to be closed. The proposed fix will not work because of this fragment in the implementation of HttpUrlConnection: Exception exc = null; try { getInputStream(); } catch (Exception e) { exc = e; } It seems as if input stream is leaked but since we are only asking for the status code is something is wrong, getInputStream() will throw an exception right away i.e. it will not create the stream and there will be nothing to close. We cannot do anything here unless somebody does further investigation and suggest a patch that clearly fixes the problem and does so using public Java API.
>The suggested fix implies reliance on the internal implementation. There is >nothing in the java.net.HTTPUrlConnection that implies that 'getStatusCode' >creates the input stream that needs to be closed. You probably meant 'getResponseCode', right? The above statement is somewhat incorrect, as the Javadoc in URLConnection.connect() says: * ... Operations that depend on being * connected, like getContentLength, will implicitly perform the * connection, if necessary. * ... Obviously getResponseCode() is one of those methods and hence (if no IOException got thrown - which was a Sun bug in the 404 case that got fixed in 1.5.0_10 and 6.0) you do know that the connection got connected by this method and you have to call HttpConnection.disconnect(). So, even if the internal impl opens the socket and catches the IOException in order to return the response code and doesn't close the socket (which was as Sun bug) you can still call disconnect() to workaround this issue without making any assumptions on internals. But just to be clear: this was a bug in Sun's VM < 5.0 and is now fixed. The problem also doesn't occur if you use e.g. an IBM VM.
OK, calling 'disconnect()' is fine. I will do that. This is what I did: 1) I added a 'close' method to the response interface (still can do that, not an API). 2) Implemented 'close' in HttpResponse as follows: public void close() { if( null != in ) { try { in.close(); } catch (IOException e) { } in = null; } if (connection!=null) { ((HttpURLConnection)connection).disconnect(); connection = null; } } 3) In checkConnectionResult, if the status code is not HTTP_OK, fetch the result and then close the response: public static void checkConnectionResult(IResponse response,URL url) throws IOException { // did the server return an error code ? int result = response.getStatusCode(); if (result != IStatusCodes.HTTP_OK) { String serverMsg = response.getStatusMessage(); response.close(); // <--- !!!! throw new FatalIOException(NLS.bind(Messages.ContentReference_HttpNok, (new Object[] { new Integer(result), serverMsg, url }))); } } I think this should do it.
