On Thu, 9 Jun 2022 11:15:14 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Conor Cleary has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8286171: Added teardown method and comments > > src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 308: > >> 306: // Have to mark request as sent, due to no request body being >> sent >> 307: // in the event of a 417 Expectation Failed >> 308: requestSent(); > > The `requestSent()` method in `Stream` class sets a flag and only closes the > Stream instance if the `responseReceived` is also set. As far as I can see in > the `Stream` code, that flag plays no other major role and doesn't "trigger" > any kind of action (I might be wrong though). Here, are we expecting the > caller client to get a request failure (just like the `HTTP/1.1` case)? If > so, should we instead of calling > `Stream.completeResponseExceptionally(Throwable)` or something similar which > completes the `CompletableFuture`? I think I see what we are doing here. The `Exchange` change that we did in this PR marks the `CompletableFuture` as completed with that response that was received. So the caller will see the future as completed. So the only missing piece for me now is, will the `Stream` get closed in this case, with the current change in the PR? ------------- PR: https://git.openjdk.java.net/jdk/pull/9093
