On Wed, 12 Oct 2022 10:54:50 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 bytes. This will cause the 
> underlying subscriber to cancel its subscription, possibly before any 
> response bytes have been received. 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.

@dfuch see

if (httpResponse.headers().firstValueAsLong("Content-Length").orElse(-1) > 
minRedAlertEventContentLength)

there are cases that the response code is 200 but for my reasons I don't want 
to read the body (for example body that contains only BOM bytes).

I guess that fixes it too 😃

thanks!!

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

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

Reply via email to