RFR: 8223714: HTTPSetAuthenticatorTest could be made more resilient

[Daniel Fuchs]

Hi Jaikiran,

1075                     System.out.println("Server will stop accepting 
connections due to an exception");
1076                     io.printStackTrace(System.out);
1077                     break;

Two things here:

  1.if ss.connect() throws, it may be because ss is closed, or not.
    To be on the safe side, we should call ss.close() before break;
  2.if the application calls stop() I expect that the call to ss.accept()
    will throw.

    Therefore I think that the exception should be simply skipped
    if stop == true (that's normal termination, we don't want to pollute
    the log with stack traces of exceptions that are expected),
    but we should probably always print it on System.err if stop == false
    otherwise we will be reduced to guessing if we see the client side
    failing with Connection Refused.

Otherwise, I believe it looks good. Thanks for another good fix :-)

best regards,

-- daniel


On 30/08/2019 13:10, Jaikiran Pai wrote:
> Can I please get a review and a sponsor for a patch for
> https://bugs.openjdk.java.net/browse/JDK-8223714?
> 
> The patch is available as a webrev at
> http://cr.openjdk.java.net/~jpai/webrev/8223714/1/webrev/
> 
> This issue and the patch relates solely to a test infrastructure class -
> HTTPTestServer and touches no main source code. The patch involves the
> following:
> 
> - A straightforward fix in the readLine method which prevents the
> java.lang.StringIndexOutOfBoundsException reported in that JIRA.
> 
> - Additional changes to the run() method of the server to make the
> connection handling and processing of this (test) server a bit more robust.
> 
>      - The run method now accepts a connection and if while processing
> the client request on that connection, runs into a IOException, then
> closes the client connection and then continues to accept any subsequent
> connections (this wasn't the case previously, where the server would
> just abort and no longer accept any more connections).
> 
>      - Any non IOException(s) are logged and the server goes on to accept
> any subsequent requests.
> 
>      - Whether or not to accept any more connections is now handled by
> the "stop" flag which gets set when the server stop is requested.
> 
>      - Any failure to "accept()" a connection stops the server from
> accepting any more requests. This is also now clearly logged as a log
> message.
> 
>      - The bulk of the parsing and processing of the client request has
> been moved out of the run() method into a new private
> processRequestAndWaitToComplete method, for better code clarity.
> 
> I haven't been able to reproduce the original exception reported in the
> JBS issue, before doing any of these changes. However, I have run the
> existing HTTPSetAuthenticatorTest, both before and after these changes
> and it has worked fine. Given that the changes are only in the test
> infrastructure, supporting the test cases, I haven't added any new jtreg
> test, to test these changes.
> 
> -Jaikiran
> 
>