On Tue, 11 Oct 2022 15:49:14 GMT, Daniel Fuchs <[email protected]> wrote:

> When [JDK-8277969](https://bugs.openjdk.org/browse/JDK-8277969) was 
> implemented, a list of outstanding response subscribers was added to 
> `HttpClientImpl`. A body subscriber is added to the list after being created 
> and is removed from the list when it is completed, either successfully or 
> exceptionally.
> 
> It appears that in the case where the subscription is cancelled before the 
> subscriber is completed, the subscriber might remain registered in the list 
> forever, or at least until the HttpClient gets garbage collected. This can be 
> easily reproduced using streaming subscribers, such as 
> BodySubscriber::ofInputStream. In the case where the input stream is closed 
> without having read all the bytes, Subscription::cancel will be called. 
> Whether the subscriber gets unregistered or not at that point becomes racy.
> 
> Indeed, the reactive stream specification doesn't guarantee whether 
> onComplete or onError will be called or not after a subscriber cancels its 
> subscription. Any cleanup that would have been performed by 
> onComplete/onError might therefore need to be performed when the subscription 
> is cancelled too.

That's exactly what this change is going to fix. What happens in the code 
you're showing here is that if the status code is not 200, then the input 
stream is closed without having read any bites. This will cause the underlying 
subscriber to cancel its subscription. And that is where the leak comes from, 
as there's no guarantee that the subscriber's onComplete/onError will ever be 
called in that case. With this fix, the subscriber will be taken off the list 
when the subscription is cancelled, and that should fix the leak.

-------------

PR: https://git.openjdk.org/jdk/pull/10659

Reply via email to