On Wed, 22 Oct 2025 12:20:41 GMT, Daniel Fuchs <[email protected]> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Replace wrapper's `preTerminationCallback` argument with a method to be >> extended > > src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java line > 414: > >> 412: return handleNoBody(r, exch); >> 413: } >> 414: return exch.readBodyAsync(responseHandler, >> this::cancelTimer) > > Could we remove the `preTerminationCallback` parameter, and instead have the > various subclass of `HttpBodySubscriberWrapper` supply > `this.multi::cancelTimer()` to their super class? > > If I'm not mistaken all concrete subclasses of `HttpBodySubscriberWrapper` > have access to the multi exchange. > > That should reduce the amount of changes significantly. > > Another possibility is to introduce a noarg protected abstract method in > `HttpBodySubscriberWrapper` and have all subclasses implement it. > We could call it for instance `protected abstract void > onTerminationNotified();` and explain that it can be used to cancel a timer > if needed. > > It might need to be called on cancel as well? > If not a comment explaining why might be good to have. If it's not called by > cancel then maybe the method suggested above could be changed to > `onCompletionNotified`. @dfuch, I've implemented the changes replacing `preTerminationCallback` argument with a `onTermination` method to be extended in `HttpBodySubscriberWrapper`. For your consideration: see [before] and [after]. [before]: https://github.com/openjdk/jdk/pull/27469/files/3b5645eb5ad79e3594767540c5fcb09b4242e3ab [after]: https://github.com/openjdk/jdk/pull/27469/files/e17131c042f6ab5292ca04eeb6f6be6d85fa784a ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2473460977
