RFR 8237858: PlainSocketImpl.socketAccept() handles EINTR incorrectly

[Alan Bateman]

On 30/06/2020 06:39, Vyom Tiwari wrote:
>
>
> Thanks for review please find the latest 
> webrev(http://cr.openjdk.java.net/~vtewari/8237858/webrev0.8/index.html). 
> I fixed the Windows build issue.
>
Minor nit but I assume build-dev will want to keep the indentation in 
TestFilesCompilation.gmk consistent. Also the patch still has the 
formatting issues in linux_close.c and bsd_close.c. You thought it might 
be webrev but looks to me that the issues are in the patch file too.

The updated tests are looking better. A few comments on 
SocketAcceptInterruptTest, most apply to SocketReadInterruptTest too:
- "service" isn't a great name for the Executor. Also you can make use 
of try-finally, e.g.
ExecutorService executor = Executors.newFixedThreadPool(1);
try { ... } finally { executor.shutdown(); }
- you can also use try-with-resources for the ServerSocket and it will 
avoid checking if ss is nul
- sendSignal - not clear why this needs the "ss" parameter, maybe to 
make the testing of the buggy implementation more robust?
- I assume thet isClosed check at L64 can be removed too.
- is Server.isSet needed - the getID method just needs to spin until 
threadId != 0.

-Alan.