Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpClient should cancel request write & response processing on unsubscription #21

Closed
NiteshKant opened this issue Jan 28, 2014 · 4 comments
Labels
Milestone

Comments

@NiteshKant
Copy link
Member

Currently HttpClient cancels the connection subscription when the caller of HttpClient.submit() unsubscribe. This in turn closes the connection.
The relevant code is here:

https://github.com/Netflix/RxNetty/blob/master/src/main/java/io/reactivex/netty/protocol/http/HttpClientImpl.java#L71

Ideally, it should do the following:

  1. If write is not completed, cancel write, which if successful should close connection.
  2. If the response is not completed, we wait for response to complete and then close the connection.

In the above statements, close the connection in presence of connection pooling (#20) would signify connection returning to pool. In such a case, the connection should be in a sane state to re-use i.e. no read/write should be pending.

@ghost ghost assigned NiteshKant Jan 28, 2014
@allenxwang
Copy link

I was under the impression that if the Observer is unsubscribed, the channel should be closed in current implementation. But it does not seem to be true. Here are the steps I observed during debugging:

  • Request is successfully written to the pipeline and response is also received in the pipeline. onCompleted() method of the Observer that is subscribed to Observable<HttpResponse> is called.
  • the above Observer is automatically unsubscribed
  • Line connectSubscription.unsubscribe(); inside HttpClientImpl.submit() is called. However, stepping into this call shows that the internal state is already "unsubscribed", so nothing really happened. Another observation is that the method ObservableConnection.close() is never invoked.

So it seems that the channel still remains open.

@NiteshKant
Copy link
Member Author

Line connectSubscription.unsubscribe(); inside HttpClientImpl.submit() is called. However, stepping into this call shows that the internal state is already "unsubscribed", so nothing really happened.

The unsubscribe from connectObservable does not close the connection as it just means, the subscriber does not want to get any new connections. The connection close is explicit, which does not seem to be happening currently in HttpClient. HttpClient should call close on the connection when the request processing is completed.
I will fix this issue.

NiteshKant pushed a commit to NiteshKant/RxNetty that referenced this issue Mar 25, 2014
allenxwang pushed a commit that referenced this issue Mar 27, 2014
@NiteshKant
Copy link
Member Author

Now RxClient does cancel all pending writes on unsubscribe (or connection close) but it does not explicitly close a pooled connection if the response is not yet over.

@NiteshKant NiteshKant added the bug label Apr 18, 2014
@NiteshKant NiteshKant removed their assignment Aug 19, 2014
@NiteshKant NiteshKant modified the milestones: 0.3.16, 0.3.15 Nov 4, 2014
@NiteshKant
Copy link
Member Author

This was fixed as part of #208 in release 0.3.15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants