Can I please get a review for this change which proposes to fix an issue in the JDK's HttpClient implementation, noted in https://bugs.openjdk.org/browse/JDK-8308144?
The HttpClient implementation internally uses `java.util.concurrent.Flow.Subscriber` API in the `jdk.internal.net.http.common.SSLFlowDelegate` class. The `SSLFlowDelegate` class at a high level is responsible for taking in SSL network data, decrypting it and passing it along as application data. It's also responsible for accepting application data, encrypting it and passing it along as SSL network data. To do so, it uses the `Flow.Subscriber` API which provide a way to control the pace of the flow of the data. The issue at hand is when the `SSLFlowDelegate` receives incoming SSL network data and it tries to decrypt it using `javax.net.ssl.SSLEngine`, the `unwrap` call on that data can return a `BUFFER_UNDERFLOW` status. `BUFFER_UNDERFLOW` implies that the incoming data that the SSLFlowDelegate attempted to decrypt did not have enough bytes to construct a packet out of it. It's then expected that the unwrap call be reattempted by receiving more incoming network data. In its current form, SSLFlowDelegate upon noticing this `BUFFER_UNDERFLOW` status immediately requests more network data so that it can reattempt the decryption and construct application data out of it. This is where the problem resides. When the `BUFFER_UNDERFLOW` state is noticed by SSLFlowDelegate, since the SSLFlowDelegate is flow controlled, it shouldn't eagerly ask for more network data. Instead it should only ask for more network data if the application data subscriber (to whom this decrypted data will be forwarded to by the SSLFlowDelegate), has expressed the desire for any application data. If there's no demand for the application data from the application data subscriber, the SSLFlowDelegate shouldn't demand more network data even if it received a `BUFFER_UNDERFLOW` status when decrypting partial incoming network data. Not honouring the absence of a demand from the application data subscriber implies that the SSLFlowDelegate will ask for more network data whenever it runs into `BUFFER_UNDERFLOW` and will keep accumulating the decrypted application data. Such accumulated data may never be used because of the lack of demand from the application data subscriber, thus unnecessarily increasing the memory consumption of the HttpClient instance(s). Credit for identifying, investigating and providing a fix for this issue goes to @zhurs. The fix was provided in https://github.com/openjdk/jdk/pull/14159. That fix also included a test case which relied on the operating system buffer sizes and was showing intermittent failures when run in our CI. So we decided to create a different test. This PR introduces a new jtreg test which reproduces the issue and verifies the fix. The test in this PR doesn't rely on any OS level buffer sizes and instead has been implemented in a way that the amount of data that gets passed into the SSLFlowDelegate as incoming network data is tightly controlled in the test itself, thus triggering the `BUFFER_UNDERFLOW` consistently. In the absence of the source fix in this PR, the test fails always on all platforms and with the fix included, the test passes on all platforms. Given the nature of this issue, the test has been documented with code comments to explain what's being done. Additionally, this PR also does some updates to the javadoc of the SSLFlowDelegate (which is an internal class) to try and make the internal terms more easier to understand and remember. ------------- Commit messages: - 8308144: Uncontrolled memory consumption in SSLFlowDelegate.Reader Changes: https://git.openjdk.org/jdk/pull/16704/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16704&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8308144 Stats: 662 lines in 3 files changed: 643 ins; 1 del; 18 mod Patch: https://git.openjdk.org/jdk/pull/16704.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16704/head:pull/16704 PR: https://git.openjdk.org/jdk/pull/16704
