RFR(s): 8228580: DnsClient TCP socket timeout
Pavel Rappo
pavel.rappo at oracle.com
Wed Sep 4 17:02:07 UTC 2019
> On 4 Sep 2019, at 13:26, Milan Mimica <milan.mimica at gmail.com> wrote:
>
> Hello
>
> Continuing in a new thread with a RFR of my own:
> http://cr.openjdk.java.net/~mmimica/8228580/webrev.00/
Here's the link to the previous discussion:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-August/061918.html
> It has the fixes applied from the first review of the patch.
>
> The test was sporadically failing not because of missing
> synchronization, but more likely because test was not waiting for the
> TCP server thread to actually start.
That Thread.sleep + AtomicReference construct seems racy (the exception reading thread might miss it). I suggest using a synchronization aid (CountDownLatch, Phaser, Semaphore, etc.) for that. Or... since you don't do anything with the accepted socket anyway you might as well not bother with creating that thread. Once the ServerSocket has been created the connection can be established even without calling `accept`. This might simplify the code quite a lot.
> Made it throw jtreg.SkippedException if TCP port is in use. We can
> just hope it does not happen too often.
We should probably pass the exception to the jtreg.SkippedException's constructor.
Why is there timeout on L45?
* @run main/timeout=60 TcpTimeout
If you're optimizing for a case where the test fails, then please don't.
-Pavel
More information about the core-libs-dev
mailing list