On Wed, 12 Oct 2022 14:23:13 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. > > Daniel Fuchs has updated the pull request incrementally with one additional > commit since the last revision: > > More cleanup to the test test/jdk/java/net/httpclient/CancelStreamedBodyTest.java line 295: > 293: uri = uri + "/testInputStream"; > 294: out.printf("%n%s testInputStream(%s, %b)%n", now(), uri, > sameClient); > 295: for (int i=0; i< ITERATION_COUNT; i++) { Should we reconsider running each test more than once? Here it looks like we are running 3 * 2 = 6 times for testing this use case. test/jdk/java/net/httpclient/CancelStreamedBodyTest.java line 407: > 405: String msg = (req != null && req.length != 0) > 406: ? new String(req, UTF_8) > 407: : BODY; As far as I can see, none of the test requests send a request body. So perhaps we can simplify this part to just always send this static `BODY` content without these additional checks? ------------- PR: https://git.openjdk.org/jdk/pull/10659
