You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Oleg Kalnichevski (JIRA)" <ji...@apache.org> on 2014/08/07 10:44:12 UTC

[jira] [Commented] (HTTPASYNC-81) Callback-Behaviour regarding connection-failures and cancellation during transfer.

    [ https://issues.apache.org/jira/browse/HTTPASYNC-81?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14089005#comment-14089005 ] 

Oleg Kalnichevski commented on HTTPASYNC-81:
--------------------------------------------

Markus
I committed your patch with some changes. Please review.

Oleg

> Callback-Behaviour regarding connection-failures and cancellation during transfer. 
> -----------------------------------------------------------------------------------
>
>                 Key: HTTPASYNC-81
>                 URL: https://issues.apache.org/jira/browse/HTTPASYNC-81
>             Project: HttpComponents HttpAsyncClient
>          Issue Type: Bug
>    Affects Versions: 4.0.1
>            Reporter: Markus Kull
>            Priority: Minor
>             Fix For: 4.0.3, 4.1-alpha1
>
>         Attachments: patch.txt
>
>
> The current behaviour regarding the callback-methods in HttpAsync-RequestProducer/ResponseConsumer seems surprising to me. I am not quite sure wether this is an actual bug or just a misunderstanding by me. Some javadocs would be helpful.
> 1) In case of unknown targethost, only close() is called on producers+consumers, but not failed() with the exception. Somehow inconvenient because the callbacks dont see the result-exception.
> {noformat}
>     @Test
>     public void testFailedBeforeClose() throws Exception {
>         CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
>         client.start();
>         HttpGet getMethod = new HttpGet("http://doesnotexist.example.org/");
>         HttpAsyncRequestProducer producer = HttpAsyncMethods.create(getMethod);
>         final AtomicBoolean closed = new AtomicBoolean(false);
>         final AtomicBoolean cancelled = new AtomicBoolean(false);
>         final AtomicBoolean failed = new AtomicBoolean(false);
>         
>         HttpAsyncResponseConsumer<String> consumer = new HttpAsyncResponseConsumer<String>() {
>             @Override
>             public void close() throws IOException {
>                 closed.set(true);
>             }
>             @Override
>             public boolean cancel() {
>                 cancelled.set(true);
>                 return false;
>             }
>             @Override
>             public void failed(Exception ex) {
>                 failed.set(true);
>             }
>             
>             public void responseReceived(HttpResponse response) throws IOException, HttpException {
>             }
>             public void consumeContent(ContentDecoder decoder, IOControl ioctrl) throws IOException {
>             }
>             public void responseCompleted(HttpContext context) {
>             }
>             public Exception getException() {
>                 return null;
>             }
>             public String getResult() {
>                 return "result";
>             }
>             @Override
>             public boolean isDone() {
>                 return false;
>             }
>         };
>         Future<String> future = client.execute(producer, consumer, null, null);
>         try {
>             future.get();
>             Assert.fail();
>         } catch (ExecutionException e) {
>             Assert.assertTrue(e.getCause() instanceof UnknownHostException);
>         }
>         Thread.sleep(1000); // give the httpclient time to clean up
>         
>         Assert.assertTrue(closed.get());
>         Assert.assertFalse(cancelled.get());
>         Assert.assertTrue(failed.get()); // FAILS! because failed() is not called. Seems to be the same for producer
>         
>         client.close();
>     }
> {noformat}
> 2) If the responseconsumer cancels the response (by isDone() returning true prematurely), then close() is never called on producers or consumers. This could become a resource leak with e.g. ZeroCopyConsumers/Producers. Connections seem to be cleaned up, though.
> {noformat}
>     @Test
>     public void testCloseOnConsumerCancel() throws Exception {
>         CloseableHttpAsyncClient client = HttpAsyncClients.createDefault();
>         client.start();
>         HttpGet getMethod = new HttpGet("http://www.example.org/");
>         HttpAsyncRequestProducer producer = HttpAsyncMethods.create(getMethod);
>         final AtomicBoolean closed = new AtomicBoolean(false);
>         final AtomicBoolean cancelled = new AtomicBoolean(false);
>         final AtomicBoolean failed = new AtomicBoolean(false);
>         
>         HttpAsyncResponseConsumer<String> consumer = new HttpAsyncResponseConsumer<String>() {
>             @Override
>             public void close() throws IOException {
>                 closed.set(true);
>             }
>             @Override
>             public boolean cancel() {
>                 cancelled.set(true);
>                 return false;
>             }
>             @Override
>             public void failed(Exception ex) {
>                 failed.set(true);
>             }
>             
>             public void responseReceived(HttpResponse response) throws IOException, HttpException {
>             }
>             public void consumeContent(ContentDecoder decoder, IOControl ioctrl) throws IOException {
>             }
>             public void responseCompleted(HttpContext context) {
>             }
>             public Exception getException() {
>                 return null;
>             }
>             public String getResult() {
>                 return "result";
>             }
>             @Override
>             public boolean isDone() {
>                 return true; // cancels fetching the response-body
>             }
>         };
>         Future<String> future = client.execute(producer, consumer, null, null);
>         future.get();
>         Thread.sleep(1000); // give the httpclient time to clean up
>         Assert.assertTrue(future.isCancelled());
>         // Assert.assertTrue(cancelled.get()); // unclear wether it should be set, because the consumer itself cancelled
>         Assert.assertFalse(failed.get()); 
>         Assert.assertTrue(closed.get()); // FAILS! the consumer wasnt closed
>         
>         client.close();
>     }
> {noformat}



--
This message was sent by Atlassian JIRA
(v6.2#6252)

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