RFR: 8229867: Re-examine synchronization usages in http and https protocol handlers [v2]
Daniel Fuchs
dfuchs at openjdk.java.net
Fri Oct 9 12:12:05 UTC 2020
On Fri, 9 Oct 2020 09:01:43 GMT, Chris Hegarty <chegar at openjdk.org> wrote:
>> Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision:
>>
>> 8229867: Re-examine synchronization usages in http and https protocol handlers
>>
>> Incorporated review feedback
>
> Mostly looks good. Just a few comments.
>From Alan:
> Allowing for visibility failures here is confusing for maintainers. If you really want to access closed without the
> lock (and I don't see any reason to do that) then it would be clearer to all if it were volatile.
>From Chris:
> This double check of closed is kind of irritating. Is it really need, or maybe we just drop it and lock unconditionally?
=> I have removed the double locking.
>From Alan:
> I don't have a strong opinion here but they did initially look like left over comments. Will they mean anything to
> someone looking at this code in 2025?
I have updated the comments to be more explanatory for future maintainer (especially if they use the Annotate feature
of the IDE to correlate with the fix in which they were introduced). That said, if you prefer removing them altogether
let me know, and I'll remove them.
-------------
PR: https://git.openjdk.java.net/jdk/pull/558
More information about the security-dev
mailing list