On Mon, 6 Dec 2021 11:30:12 GMT, Aleksey Shipilev <[email protected]> wrote:
>> There are several thread safety issues in java.net.ServerSocket, issues that
>> go back to at least JDK 1.4.
>>
>> The issue of most concern is async close of a ServerSocket that is initially
>> created unbound and where close may be called at or around the time the
>> underlying SocketImpl is created or the socket is bound.
>>
>> The summary of the changes are:
>>
>> 1. The "impl" field is changed to be final field.
>> 2. The closeLock is renamed to stateLock and is required to change the (now
>> volatile) created, bound or closed fields.
>> 3. The needless synchronization has been removed from xxxSoTimeout and
>> xxxReceiveBufferSize.
>>
>> There are many redundant checks for isClosed() and other state that could be
>> removed. Removing them would subtle change the exception thrown when there
>> are two or more failure conditions. So they are left as is.
>
> 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.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6712
