> Can I please get a review of this test-only change which addresses an > intermittent failure in `test/jdk/com/sun/net/httpserver/Test12.java`? This > fixes https://bugs.openjdk.org/browse/JDK-8359477. > > In the linked JBS issue I've added a comment which explains what causes this > intermittent failure. The issue is a bug/race within the test. There are 2 > commits in this PR and only of them is necessary for addressing this issue. > The other one is a general cleanup of the test. I am willing to revert the > general cleanup commit from this test if others feel that it shouldn't be > done in this PR. > > The actual fix is this commit > https://github.com/openjdk/jdk/commit/a0f8fc806e1682ef909cb7b4a449be855072fe48 > which stops deleting the input files upon test completion. That should be OK > because the jtreg test infrastructure will clean them up from the scratch > directory after the completion of the test run (if the test succeeds). There > are other ways to address this race condition and continue to delete these > files (for example, co-ordinating between the test and the test specific > `FileServerHandler`), but to me it didn't seem worthwhile doing that. > > I've run this change around a 1000 times and haven't seen it fail. > > P.S: The `test/jdk/com/sun/net/httpserver/Test1.java` does the same things as > this test, but does it sequentially. I'm pretty sure that we should be able > to trigger this intermittent test failure in that test too if I reorder the > `test()` calls in that test to have a fixed length test be the last one to > execute (i.e. `test (true, ...)` instead of the current `test(false, ...)`).
Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision: - use ExecutorService.close() instead of ExecutorService.shutdown() to wait for the server side handlers to complete execution - use try-with-resource in test's FileServerHandler - Revert "8359477: com/sun/net/httpserver/Test12.java appears to have a temp file race" This reverts commit a0f8fc806e1682ef909cb7b4a449be855072fe48. - merge latest master branch changes - minor change to test's log message - add a log message in the test - 8359477: com/sun/net/httpserver/Test12.java appears to have a temp file race - 8359477: general cleanup of the test ------------- Changes: - all: https://git.openjdk.org/jdk/pull/25820/files - new: https://git.openjdk.org/jdk/pull/25820/files/42b86dee..7f03aa43 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25820&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25820&range=02-03 Stats: 21265 lines in 805 files changed: 9166 ins; 7793 del; 4306 mod Patch: https://git.openjdk.org/jdk/pull/25820.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25820/head:pull/25820 PR: https://git.openjdk.org/jdk/pull/25820
