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.
