On Mon, 6 Dec 2021 11:40:46 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> src/java.base/share/classes/java/net/ServerSocket.java line 90:
>> 
>>> 88:     private volatile boolean created;   // impl.create(boolean) called
>>> 89:     private volatile boolean bound;
>>> 90:     private volatile boolean closed;
>> 
>> I don't see why these two need to be `volatile`, but we can keep them for 
>> extra safety.
>
> isBound and isClose poll the state.

Ah yes, I see. Doing `while (!socket.isClosed()) ...` would be problematic 
otherwise.

>> src/java.base/share/classes/java/net/ServerSocket.java line 804:
>> 
>>> 802:      * @see #setSoTimeout(int)
>>> 803:      */
>>> 804:     public int getSoTimeout() throws IOException {
>> 
>> Is it safe to drop `synchronized` here? Any subclasses/implementations rely 
>> on mutual exclusion here?
>
> It would be relying on undocumented and inconsistent behavior. I think they 
> are left over from when the getter/setter methods were synchronized with 
> close or the old socket implementation where the Socket and SocketImpl 
> implementation were very tangled. Methods such as xxxReuseAddress don't 
> synchronize, another inconsistency. I can drop the removal of synchronized 
> from the patch but it really don't serve any purpose here.

All right then.

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

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

Reply via email to