RFR: 8229867: Re-examine synchronization usages in http and https protocol handlers

[Alan Bateman]

On Thu, 8 Oct 2020 17:36:31 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

> Mostly that was a mechanical change since openServer was synchronized before.
> But I guess it seems also desirable for accessing host & port which are protected and not final;

Okay, I mis-read the old code. The syncrhonization to access host/port isn't obvious so leaving it as is is okay.
 
> I am not so sure. `closed` starts at `false`, and can only moved from `false` to `true`. It can never go back to
> `false` after it's been set to `true`; So if you observe `true` outside of the lock it does means that it is really
> closed, isn't it? If `closed` is not volatile, the worse that can happen is that you will observe `false` while it's
> really `true`, and then you will need to pay the price of locking, after which you should be able to see the real
> `true` value.

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.


> Yes - well - I thought that would be mostly beneficial for someone searching for `synchronized` in the java/net and
> sun/net packages - so that they can determine that the `synchronized` in those places was considered safe and
> intentionally kept unchanged, and not just overlooked. I can remove them or reformulate if you think it's better - but
> I was actually intending to keep these comments.

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?

-------------

PR: https://git.openjdk.java.net/jdk/pull/558