RFR: 8336815: Socket.bind/connect and ServerSocket.bind/accept do not specify behavior when already bound, connected or closed [v4]
Alan Bateman
alanb at openjdk.org
Sun Jul 21 06:42:41 UTC 2024
On Sat, 20 Jul 2024 16:48:54 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> I've updated the PR to remove the usages of the parameterized test and instead inline the operations within the test itself. The tests continue to pass.
>
>> I think it would be better to not repeat the "if" in each of the thrown, e.g. "if the bind operation fails, the socket is already bound, or the socket is closed".
>
> Done - I've removed the repeated "if" from the throws clause.
>
>> some of the socket options have "an error in the underlying protocol, such as a TCP error" but none of these methods are doing I/O, they are just setting or reading socket options. They should say "if there is an error SO_RCVBUF" or whatever. It's not important here, it's just jumping out when reading these old descriptions.
>
> I decided to leave them as-is in this PR, given how many of those there are. I think we can decide to clean them up in a separate PR if needed.
>
>> You asked about specifying the more specific SocketException. These APIs are pluggable via SocketImpl so specifying a more specific exception could potentially invalidate existing implementations. I don't know if there are any other implementations in 2024 but I think changing anything here would require further thought on the compatibility impact.
>
> Given that context, leaving them in their current form sounds reasonable to me.
The updated tests looks okay.
No action required but just to say that future work could expand the test for when socket is bound before closed or connected before closed, thus ensure the methods have the specified behavior for all possible states. For the method that don't throw then it would be possible to assert the return values.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20268#discussion_r1685660996
More information about the net-dev
mailing list