RFR: 8223714: HTTPSetAuthenticatorTest could be made more resilient

[Jaikiran Pai]

Hello Vyom, Daniel,

On 02/09/19 3:00 PM, Daniel Fuchs wrote:
> Hi,
>
> On 02/09/2019 05:29, Vyom Tewari26 wrote:
>>>     2-> i think super.stop() should be the first line in the
>>>     overridden stop method.
>>
>>     I had thought of this when I introduced the change. However, I felt
>>     that it's better to set the stop=true before calling the
>>     super.stop(), since this way the other thread gets "notified" a bit
>>     early that it doesn't have to accept any more connections, thus it
>>     doesn't have to first go into a blocking accept() and then get
>>     thrown an exception (when the stop() method does a ss.close()) and
>>     then go and check if it was asked to stop. Of course, this doesn't
>>     guarantee that the other thread will always benefit from this (since
>>     it might already be in a blocking accept()), but at least it does
>>     present the other thread a chance >
>> My only worry was what if super.stop() fails and throws exception but
>> in super.stop() we are catching/ignoring the exception and logging
>> the trace to system.out.
>
> I'd also prefer setting `stop` to  true before calling super.stop() for
> the reason that Jaikiran states. That said maybe a better implementation
> could be:
>
>     try (var toClose = ss;) {
>         stop = true;
>         super.stop();
>     } catch (IOException ex) {
>         ...
>     }
>
> this way we can be sure that ss.close will be closed
> even if super.stop() throws something (which shouldn't
> happen but who knows?)

This does look better. I have updated the code to use this construct and
the new webrev is at
http://cr.openjdk.java.net/~jpai/webrev/8223714/3/webrev/ (only the
stop() method code has changed compared to previous webrev).

-Jaikiran