RFR: 8359477: com/sun/net/httpserver/Test12.java appears to have a temp file race [v3]
Volkan Yazici
vyazici at openjdk.org
Mon Jun 16 09:52:33 UTC 2025
On Mon, 16 Jun 2025 09:29:38 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> 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.
> 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
@jaikiran, `executor` is configured for the servers via `s1.setExecutor (executor)` and `s2.setExecutor (executor)` lines above. I was under the impression that shutting down the executor (i.e., interrupting the server handlers) would result in server releasing all acquired resources, including issuing a `FIS::close`. Was I mistaken?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25820#discussion_r2149527378
More information about the net-dev
mailing list