[9] RFR: 8164166: Make sure java/nio/channels tests shutdown asynchronous channel groups

[Artem Smotrakov]

Thank you for review Alan. Please see inline.


On 08/23/2016 02:50 AM, Alan Bateman wrote:
> On 23/08/2016 01:28, Artem Smotrakov wrote:
>
>> :
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164166
>> Webrev: http://cr.openjdk.java.net/~asmotrak/8164166/webrev.00/
> For test/java/nio/channels/AsynchronousChannelGroup/Basic.java then it 
> would be a lot cleaner to put the cleanup in shutdownTests, e.g.:
>
> AsynchronousChannelGroup group = ...
> try {
>     testShutdownWithNoChannels(pool, group);
> } finally {
>     group.shutdown();
> }
testShutdownWithNoChannels() method calls group.shutdown() right away, 
so that it doesn't look necessary to use try-finally block here.

I added try-finally blocks to testShutdownWithChannels() and 
testShutdownNow() methods to reduce a number of try-finally blocks. I 
can wrap each call of testShutdownWithChannels() and testShutdownNow() 
methods to try-finally block if you think it is cleaner.
>
> In GroupOfOne then L47-49 isn't early to read. If you move the bind 
> into the try-finally, as in: listener.bind(new InetSocketAddress(0))) 
> then it would be a bit easier. Same comment for the changes to 
> Identify and testRestart in Restart.java.
Okay, I'll split it.
>
> In testRestart then it looks like the try is in the wrong place, I 
> assume you want:
>
> group = ...
> try {
>     testRestart(...)
> } finally {
>     group.shutdown();
> }
Right. I'll change it.

Here is an updated webrev

http://cr.openjdk.java.net/~asmotrak/8164166/webrev.01/

Artem
>
> -Alan
>
>
>