On Thu, 13 Jan 2022 04:52:25 GMT, Jaikiran Pai <[email protected]> wrote:
>> Julia Boes has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> address PR comments:
>> * remove redundant null check
>> * use try-finally to stop process in test
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 525:
>
>> 523: logger.log(Level.TRACE, "exchange started");
>> 524:
>> 525: if (dispatcherThread != null && dispatcherThread ==
>> Thread.currentThread()) {
>
> Hello Julia,
> Minor thing - With the `dispatcherThread == Thread.currentThread()` check,
> the `dispatcherThread != null` would be redundant and perhaps can be removed?
You're right, removed.
> test/jdk/com/sun/net/httpserver/simpleserver/jwebserver/MaxRequestTimeTest.java
> line 104:
>
>> 102: sendHTTPRequest(); // server expected to respond successfully
>> 103:
>> 104: p.destroy();
>
> Since the `sendXXXRequest()` methods above could potentially throw
> exceptions, do you think it would be better to destroy the process in a
> finally block? Or maybe manage the process start and destroy as testng
> before/after lifecycle methods?
Good point, I'll update that. Thanks!
-------------
PR: https://git.openjdk.java.net/jdk/pull/7052
