On Mon, 20 Oct 2025 11:10:50 GMT, Mark Sheppard <[email protected]> wrote:

>> test/jdk/com/sun/net/httpserver/ServerStopTerminationTest.java line 164:
>> 
>>> 162:         // Complete the exchange 10 second into the future.
>>> 163:         // Runs in parallel, so won't block the server stop
>>> 164:         final Duration exchangeDuration = 
>>> Duration.ofSeconds(Utils.adjustTimeout(10));
>> 
>> Looks good, but if this test fail we could envisage bumping that delay 
>> though. A better implementation could be to complete the exchange *after* 
>> exiting from server.stop() - without using any virtual thread, and just 
>> verifying that the server.stop() waited at least for 1s.
>
> The second temporal constraint is something that could possibly fail. If it 
> fails why is it considered a test failure?
> 
> Consider a slightly convoluted scenario:
> 
> The Exchange will activate after 1 second. That thread is scheduled to 
> execute but remains RTR due to excessive load on the system
> The server.stop is invoked with a 20 second delay, indicating this is the max 
> allowed time for graceful shutdown i.e. the time in which any extant
> exchanges should complete. If that timeout expires in the finishLatch.await 
> returns — no check is made to determine if this was a timeout expiry or not —
> then the server proceeds to shutdown.
> 
> Thus, in this scenario, the test will fail. As such, how is this a failure? 
> Your test has placed a temporal condition which has a possibility of not 
> being met.
> Your expectation is that because the shutdown delay is 20 seconds the test 
> should always pass, with the Exchange having completed, as such 
> deterministic. This
> is not unreasonable, but it is not a condition that can be 100% met, and is 
> not deterministic.
> 
> The semantics of server.stop doesn’t consider a delay timeout as an error 
> condition, so why is this the case in the test.
> So rather than it being a test failure, should a delay expiry just be 
> observational ?

Was that a comment for  the other test @msheppar  - there's no 20s here.
I still think a better impl would be to call completeExchange() after 
timeShutdown() here since we want to verify that the server will stop after the 
1s delay even if an exchange is in progress.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/27670#discussion_r2445569883

Reply via email to