On Tue, 4 May 2021 16:07:42 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Fernando Guallini has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> var volatile
>
> test/jdk/sun/security/ssl/SSLSocketImpl/CloseSocket.java line 45:
>
>> 43: public class CloseSocket extends SSLSocketTemplate {
>> 44:
>> 45: private static Thread clientThread = null;
>
> Shouldn't this variable be `volatile`? If I'm not mistaken it's set in one
> thread and potentially read in a different thread? An alternative could be to
> use a CountDownLatch instead.
Thanks, updated to be volatile.
SSLSocket::startHandshake internally first checks that the socket is not closed
or broken and still connected, so it needs the server to close the socket after
those verifications are performed to reproduce the test scenario, thus a
CountDownLatch in the test before calling startHandshake would not guarantee
that its internal operations are run before the server, already unblocked at
that time, closes the socket
A CountDownLatch after startHandshake does not work either since the client
keeps waiting for a server response, which is blocked waiting for the latch.
That is why I think that looking at the thread stack is the best way to
guarantee the scenario is properly verified
-------------
PR: https://git.openjdk.java.net/jdk/pull/3856
