On Fri, 2 Jun 2023 08:25:35 GMT, Jaikiran Pai <[email protected]> wrote:
>> Daniel Fuchs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review feedback
>
> src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java line 176:
>
>> 174: } else {
>> 175: this.connection = null;
>> 176: this.cause = null;
>
> Hello Daniel, I had to read this a few times to understand why we are
> resetting `this.cause`, which we potentially set a few lines above, to `null`
> here. From what I understand, we only want to set `this.cause` when the
> `connection` is `null`. Do you think this code could instead of written as:
>
>
> Throwable cause;
> synchronized (this) {
> cause = this.cause;
> if (cause == null) {
> cause = error;
> }
> connection = this.connection;
> if (connection == null) {
> closeRequested = true;
> this.cause = cause;
> } else {
> this.connection = null;
> this.cause = null;
> }
> }
Much better yes. I'll steal it!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14251#discussion_r1214212516
