On Fri, 9 Oct 2020 09:01:43 GMT, Chris Hegarty <che...@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
