RFR: 8317522: Test logic for BODY_CF in AbstractThrowingSubscribers.java is wrong

Daniel Fuchs dfuchs at openjdk.org
Wed Oct 4 17:22:51 UTC 2023


The `AbstractThrowingSubscribers.java` test class makes the assumption that the HttpClient will cancel a request if the body `CompletableFuture` returned by a `BodySubscriber::getBody` is completed exceptionally. This assumption is wrong. Indeed, it should be the responsibility of the custom subscriber to cancel its subscription, or not, in such a case, as whether to cancel or not depends on the semantic the subscriber assigns to such a body.

The issue can be observed by modifying the test to send a longer response split in several chunks: with such a configuration, and using streaming body subscribers, such as `ofInputStream`, the test will fail waiting for the HttpClient to be garbaged collected, as the response data will not get pulled and the underlying exchange will never terminate. Though changing the HttpClient code to forcefully cancel the subscription (or the exchange) in such a case would "work", it does not seem desirable: it would introduce a change of behavior that might cause existing applications to fail, and there may be cases where a custom subscriber may legitimately want to complete its body CF exceptionally while still continuing to parse the body bytes. Whether to choose to cancel the subscription or not should solely depend on the logic of the custom subscriber.

The test logic is now modified to reflect this. Commenting the line that cancels the subscription in `ThrowingBodySubscriber::getBody` would now make the test fail. The test was previously passing "by chance" because the whole response was contained in a single `DataFrame` (or `ByteBuffer` for HTTP/1.1).

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

Commit messages:
 - 8317522

Changes: https://git.openjdk.org/jdk/pull/16041/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16041&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8317522
  Stats: 82 lines in 1 file changed: 55 ins; 11 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/16041.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16041/head:pull/16041

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


More information about the net-dev mailing list