On Mon, 16 Jun 2025 09:22:13 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> test/jdk/com/sun/net/httpserver/Test12.java line 113: >> >>> 111: executor.shutdown (); >>> 112: Files.delete(smallFilePath); >>> 113: Files.delete(largeFilePath); >> >> Instead of removing the file cleanup, IMHO, _good practice_, can't we use >> `shutdown() + awaitTermination()` or `shutdownNow()` for the executor? > > Hello Volkan, the issue here's isn't about not waiting for the tasks to > complete. They in fact do complete. The actual issue is that the tasks have > no way to know if the server side has actually done a > `FileInputStream.close()` for the fixed length case, because the > `InputStream` handed out to those tasks doesn't wait to receive a EOF from > the server side for the fixed length case. Instead since the InputStream > knows how many bytes to expect, once it receives those many bytes, it > immediately returns. The server side (in FileServerHandler) would have > transmitted those bytes and would be in the process of closing the > FileInputStream, but the client side task would already be completed. So > adding a `awaitTermination()` on the executor will not help. > Instead of removing the file cleanup, IMHO, good practice They do get cleaned up - jtreg cleans up the scratch directories (unless asked not to do so) when the test completes. The general pattern we use in JDK tests is to let jtreg clean them up for us so that if the test has failed (for whatever reason), then due to the use of `-retain:fail,error`, these input files are retained by jtreg so that they can help debug any unexpected test failures. In this case, the input files themselves very likely won't play a role in any unexpected failures. So if we still prefer to delete these explicitly in the test, then I can fix this in a different way - that will require a bit more code for handshake between the test's `FileServerHandler` and the test itself to ensure that the test knows when it is OK to delete these files. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25820#discussion_r2149483105
