From fcece026468fd52fc531033bbfb4c1156767c7eb Mon Sep 17 00:00:00 2001 From: Noah Levitt Date: Sun, 30 Dec 2012 20:45:05 -0800 Subject: [PATCH] Close recorders on failure to avoid exception from retry cycle within httpclient- now all the FetchHTTPTests pass --- .../org/archive/io/RecordingInputStream.java | 6 ++-- httpcomponents.diff | 32 +++++++++---------- .../archive/modules/fetcher/FetchHTTP.java | 16 +++------- .../modules/fetcher/FetchHTTPRequest.java | 16 ++++++++++ .../modules/fetcher/FetchHTTPTests.java | 1 + 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/commons/src/main/java/org/archive/io/RecordingInputStream.java b/commons/src/main/java/org/archive/io/RecordingInputStream.java index cf5c6d28..3c4af8fd 100644 --- a/commons/src/main/java/org/archive/io/RecordingInputStream.java +++ b/commons/src/main/java/org/archive/io/RecordingInputStream.java @@ -137,11 +137,11 @@ public class RecordingInputStream public void close() throws IOException { if (logger.isLoggable(Level.FINE)) { - logger.fine(Thread.currentThread().getName() + " closing " + - this.in + ", " + Thread.currentThread().getName()); + logger.fine("closing " + this.in + " in thread " + + Thread.currentThread().getName()); } IOUtils.closeQuietly(this.in); - this.in = null; + this.in = null; IOUtils.closeQuietly(this.recordingOutputStream); } diff --git a/httpcomponents.diff b/httpcomponents.diff index a278ab39..3865ca66 100644 --- a/httpcomponents.diff +++ b/httpcomponents.diff @@ -269,7 +269,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/client/methods/BasicA +} Index: httpclient/httpclient/src/main/java/org/apache/http/client/methods/HttpRequestBase.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/client/methods/HttpRequestBase.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/client/methods/HttpRequestBase.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/client/methods/HttpRequestBase.java (working copy) @@ -27,20 +27,12 @@ @@ -498,7 +498,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/client/methods/HttpRe } Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java (working copy) @@ -37,6 +37,7 @@ import org.apache.http.HttpHost; @@ -536,7 +536,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/AbstractH throw new ClientProtocolException(httpException); Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/DefaultUserTokenHandler.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/client/DefaultUserTokenHandler.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/client/DefaultUserTokenHandler.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/client/DefaultUserTokenHandler.java (working copy) @@ -76,7 +76,7 @@ @@ -549,7 +549,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/DefaultUs userPrincipal = sslsession.getLocalPrincipal(); Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/HttpClientBuilder.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/client/HttpClientBuilder.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/client/HttpClientBuilder.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/client/HttpClientBuilder.java (working copy) @@ -163,7 +163,7 @@ private ServiceUnavailableRetryStrategy serviceUnavailStrategy; @@ -587,7 +587,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/HttpClien } Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/InternalHttpClient.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/client/InternalHttpClient.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/client/InternalHttpClient.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/client/InternalHttpClient.java (working copy) @@ -171,6 +171,9 @@ config = ((Configurable) request).getConfig(); @@ -601,7 +601,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/client/InternalH if (config == null) { Index: httpclient/httpclient/src/main/java/org/apache/http/impl/conn/DefaultClientConnectionFactory.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/conn/DefaultClientConnectionFactory.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/conn/DefaultClientConnectionFactory.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/conn/DefaultClientConnectionFactory.java (working copy) @@ -34,8 +34,10 @@ @@ -636,7 +636,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/conn/DefaultClie } Index: httpclient/httpclient/src/main/java/org/apache/http/impl/conn/SocketClientConnectionImpl.java =================================================================== ---- httpclient/httpclient/src/main/java/org/apache/http/impl/conn/SocketClientConnectionImpl.java (revision 1426847) +--- httpclient/httpclient/src/main/java/org/apache/http/impl/conn/SocketClientConnectionImpl.java (revision 1427037) +++ httpclient/httpclient/src/main/java/org/apache/http/impl/conn/SocketClientConnectionImpl.java (working copy) @@ -50,11 +50,12 @@ import org.apache.http.conn.SocketClientConnection; @@ -692,7 +692,7 @@ Index: httpclient/httpclient/src/main/java/org/apache/http/impl/conn/SocketClien @Override Index: httpclient/httpclient/src/test/java/org/apache/http/impl/conn/SessionInputBufferMock.java =================================================================== ---- httpclient/httpclient/src/test/java/org/apache/http/impl/conn/SessionInputBufferMock.java (revision 1426847) +--- httpclient/httpclient/src/test/java/org/apache/http/impl/conn/SessionInputBufferMock.java (revision 1427037) +++ httpclient/httpclient/src/test/java/org/apache/http/impl/conn/SessionInputBufferMock.java (working copy) @@ -51,7 +51,11 @@ final MessageConstraints constrains, @@ -709,7 +709,7 @@ Index: httpclient/httpclient/src/test/java/org/apache/http/impl/conn/SessionInpu public SessionInputBufferMock( Index: httpcore/httpcore/src/main/java/org/apache/http/impl/BHttpConnectionBase.java =================================================================== ---- httpcore/httpcore/src/main/java/org/apache/http/impl/BHttpConnectionBase.java (revision 1426847) +--- httpcore/httpcore/src/main/java/org/apache/http/impl/BHttpConnectionBase.java (revision 1427037) +++ httpcore/httpcore/src/main/java/org/apache/http/impl/BHttpConnectionBase.java (working copy) @@ -55,9 +55,11 @@ import org.apache.http.impl.io.ChunkedOutputStream; @@ -753,7 +753,7 @@ Index: httpcore/httpcore/src/main/java/org/apache/http/impl/BHttpConnectionBase. this.incomingContentStrategy = incomingContentStrategy != null ? incomingContentStrategy : Index: httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpClientConnection.java =================================================================== ---- httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpClientConnection.java (revision 1426847) +--- httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpClientConnection.java (revision 1427037) +++ httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpClientConnection.java (working copy) @@ -47,6 +47,7 @@ import org.apache.http.impl.entity.StrictContentLengthStrategy; @@ -802,7 +802,7 @@ Index: httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpClientCo protected void onResponseReceived(final HttpResponse response) { Index: httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpServerConnection.java =================================================================== ---- httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpServerConnection.java (revision 1426847) +--- httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpServerConnection.java (revision 1427037) +++ httpcore/httpcore/src/main/java/org/apache/http/impl/DefaultBHttpServerConnection.java (working copy) @@ -94,7 +94,7 @@ final HttpMessageWriterFactory responseWriterFactory) { @@ -896,7 +896,7 @@ Index: httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionBufferImpl +} Index: httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionInputBufferImpl.java =================================================================== ---- httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionInputBufferImpl.java (revision 1426847) +--- httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionInputBufferImpl.java (revision 1427037) +++ httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionInputBufferImpl.java (working copy) @@ -60,14 +60,14 @@ @NotThreadSafe @@ -936,7 +936,7 @@ Index: httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionInputBuffe int len = this.linebuffer.length(); Index: httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionOutputBufferImpl.java =================================================================== ---- httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionOutputBufferImpl.java (revision 1426847) +--- httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionOutputBufferImpl.java (revision 1427037) +++ httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionOutputBufferImpl.java (working copy) @@ -64,7 +64,7 @@ private final int minChunkLimit; @@ -958,7 +958,7 @@ Index: httpcore/httpcore/src/main/java/org/apache/http/impl/io/SessionOutputBuff Index: httpcore/httpcore/src/test/java/org/apache/http/impl/SessionInputBufferMock.java =================================================================== ---- httpcore/httpcore/src/test/java/org/apache/http/impl/SessionInputBufferMock.java (revision 1426847) +--- httpcore/httpcore/src/test/java/org/apache/http/impl/SessionInputBufferMock.java (revision 1427037) +++ httpcore/httpcore/src/test/java/org/apache/http/impl/SessionInputBufferMock.java (working copy) @@ -51,7 +51,11 @@ final MessageConstraints constrains, @@ -975,7 +975,7 @@ Index: httpcore/httpcore/src/test/java/org/apache/http/impl/SessionInputBufferMo public SessionInputBufferMock( Index: httpcore/httpcore/src/test/java/org/apache/http/impl/SessionOutputBufferMock.java =================================================================== ---- httpcore/httpcore/src/test/java/org/apache/http/impl/SessionOutputBufferMock.java (revision 1426847) +--- httpcore/httpcore/src/test/java/org/apache/http/impl/SessionOutputBufferMock.java (revision 1427037) +++ httpcore/httpcore/src/test/java/org/apache/http/impl/SessionOutputBufferMock.java (working copy) @@ -28,6 +28,7 @@ package org.apache.http.impl; @@ -1000,7 +1000,7 @@ Index: httpcore/httpcore/src/test/java/org/apache/http/impl/SessionOutputBufferM Index: httpcore/httpcore/src/test/java/org/apache/http/testserver/LoggingBHttpClientConnection.java =================================================================== ---- httpcore/httpcore/src/test/java/org/apache/http/testserver/LoggingBHttpClientConnection.java (revision 1426847) +--- httpcore/httpcore/src/test/java/org/apache/http/testserver/LoggingBHttpClientConnection.java (revision 1427037) +++ httpcore/httpcore/src/test/java/org/apache/http/testserver/LoggingBHttpClientConnection.java (working copy) @@ -66,7 +66,7 @@ final HttpMessageParserFactory responseParserFactory) { diff --git a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTP.java b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTP.java index 6731d7f8..1fef38ec 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTP.java +++ b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTP.java @@ -648,10 +648,10 @@ public class FetchHTTP extends Processor implements Lifecycle { response = req.execute(); addResponseContent(response, curi); } catch (ClientProtocolException e) { - failedExecuteCleanup(req.request, curi, e); + failedExecuteCleanup(curi, e); return; } catch (IOException e) { - failedExecuteCleanup(req.request, curi, e); + failedExecuteCleanup(curi, e); return; } @@ -684,9 +684,6 @@ public class FetchHTTP extends Processor implements Lifecycle { rec.close(); // ensure recording has stopped rec.closeRecorders(); - if (!req.request.isAborted()) { - req.request.reset(); - } // Note completion time curi.setFetchCompletedTime(System.currentTimeMillis()); @@ -991,17 +988,14 @@ public class FetchHTTP extends Processor implements Lifecycle { * * @param curi * CrawlURI we failed on. - * @param request - * Method we failed on. * @param exception * Exception we failed with. */ - protected void failedExecuteCleanup(final AbortableHttpRequestBase request, - final CrawlURI curi, final Exception exception) { + protected void failedExecuteCleanup(final CrawlURI curi, + final Exception exception) { cleanup(curi, exception, "executeMethod", S_CONNECT_FAILED); - request.reset(); } - + /** * Cleanup after a failed method execute. * diff --git a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java index 72d04ee7..9a9932ed 100644 --- a/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java +++ b/modules/src/main/java/org/archive/modules/fetcher/FetchHTTPRequest.java @@ -134,6 +134,22 @@ public class FetchHTTPRequest { super.receiveResponseEntity(response); } } + + @Override + public void shutdown() throws IOException { + super.shutdown(); + + /* + * Need to do this to avoid "java.io.IOException: RIS already open" + * on urls that are retried within httpcomponents. Exercised by + * FetchHTTPTests.testNoResponse() + */ + Recorder recorder = Recorder.getHttpRecorder(); + if (recorder != null) { + recorder.close(); + recorder.closeRecorders(); + } + } } /** diff --git a/modules/src/test/java/org/archive/modules/fetcher/FetchHTTPTests.java b/modules/src/test/java/org/archive/modules/fetcher/FetchHTTPTests.java index 55f194e6..1883c483 100644 --- a/modules/src/test/java/org/archive/modules/fetcher/FetchHTTPTests.java +++ b/modules/src/test/java/org/archive/modules/fetcher/FetchHTTPTests.java @@ -787,6 +787,7 @@ public class FetchHTTPTests extends ProcessorTestBase { } } + // Implicitly tests the retry cycle within httpcomponents public void testNoResponse() throws Exception { NoResponseServer noResponseServer = new NoResponseServer("localhost", 7780); noResponseServer.start();