On Thu, 8 Dec 2022 18:28:43 GMT, Daniel Fuchs <[email protected]> wrote:

> There are several ways by which an HTTP/2 stream can be closed. Due to the 
> built-in asynchronous behavior, and to avoid unnecessary churn when a request 
> is cancelled/aborted there are two places where the boolean stream's state 
> closed can be set to true: cancelImpl and close. The current code completes 
> the subscriber in cancelImpl, but the subscriber should also be completed in 
> close. The intermittent failure happens if close gets ever called before 
> cancelImpl.
> This patch fixes that.

src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 1360:

> 1358:         if (s instanceof Http2StreamResponseSubscriber<?> sw) {
> 1359:             if (debug.on()) debug.log("closing response subscriber 
> stream %s", streamid);
> 1360:             sw.streamClosed(errorRef.get());

Given the number of different methods we have and the potential confusion it 
can cause about which one to use when, do you think in this case we could just 
reuse the existing ones instead of this new one? Something like:

if (s instanceof HttpBodySubscriberWrapper sw) {
....
  if (!sw.completed()) {
     var cause = errorRef.get();
     var ex = cause == null
            ? new IOException("stream closed")
            : cause;
     complete(ex);
  }
}

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

PR: https://git.openjdk.org/jdk20/pull/3

Reply via email to