RFR: 8278270: ServerSocket is not thread safe

[Aleksey Shipilev]

On Mon, 6 Dec 2021 11:40:46 GMT, Alan Bateman <alanb at 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