On 29/04/2019, 12:17, Alan Bateman wrote:
On 29/04/2019 10:52, Michael McMahon wrote:
Hi,

This is another change which is part of the general cleanup of SocketImpls. The change removes support for pre JDK 1.4 socketimpls which do not implement
the timed connect method. These SocketImpls have not been compilable
since 1.4 and limited runtime support has been provided since then, which is now being removed.

Webrev
-------
http://cr.openjdk.java.net/~michaelm/8216978/webrev.1/

This is a good cleanup.

Changing SIS.close and SOS.close to caller super.close raises a number of questions. These close should never be called Socket.getInputStream and getOutputStream don't leak these streams to user code (they used to but now in JDK 13). My concern is that if they were ever to be called then it will be calling the FIS/FOS close methods which brings along a several questions on it interacts with the cleaner mechanism used by those classes.

I don't think AbstractPlainSocketImpl.isBound needs to be volatile as it is guarded by the synchronization on the impl (the doConnect and bind methods are synchronized).

ok
The Windows version of PlainSocketImpl shouldn't need the constructor to be public (I see the Unix version is package-private).

ok
Should we use the opportunity to add something to the javadoc for the SocketImpl constructor so it's not empty? It can be as simple as "Initialize a new instance of this class" as used in other places where there isn't anything to say.

The empty comment block looks odd I admit. It was needed to make it build and I left it empty so that the generated apidoc would not change. However, I guess I can add the above suggestion to the CSR

Thanks,

Michael

Reply via email to