On Thu, 13 Jan 2022 04:52:25 GMT, Jaikiran Pai <j...@openjdk.org> 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

Reply via email to