On Tue, 14 Jan 2025 20:45:13 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Make `LimitingSubscriber` stateful
>> - Improve Javadoc
>> - Replace `IllegalStateException` with an `IOException`
>> - Remove `discardExcess` argument
>
> src/java.net.http/share/classes/java/net/http/HttpResponse.java line 1400:
>
>> 1398: */
>> 1399: public static <T> BodySubscriber<T> limiting(BodySubscriber<T>
>> downstreamSubscriber, long capacity) {
>> 1400: Objects.requireNonNull(downstreamSubscriber,
>> "downstreamSubscriber");
>
> Strange to check for null but not for capacity < 0 ?
Right. Fixed in c5186ba2c0ff11ac90ef00d90abe1d9a095f7185.
> src/java.net.http/share/classes/jdk/internal/net/http/LimitingSubscriber.java
> line 55:
>
>> 53: private final AtomicReference<State> stateRef = new
>> AtomicReference<>();
>> 54:
>> 55: private long length;
>
> should be volatile or AtomicLong
Fixed in 29174d163fbc85d8c308eb757edcfa4ba9ef3942.
> src/java.net.http/share/classes/jdk/internal/net/http/LimitingSubscriber.java
> line 107:
>
>> 105:
>> 106: // Otherwise, trigger failure
>> 107: else if (stateRef.compareAndSet(subscribed,
>> State.Terminated.INSTANCE)) {
>
> I would prefer to keep the regular style:
>
>
> if (condition) {
> // do something
> } else {
> // do something else
> }
Fixed in bb577e9ba8a6ce8ee24e8371c4e34aeaceeb6808.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149415
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149729
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149952
