RFR: 8252883: AccessDeniedException caused by delayed file deletion on Windows
Hi, Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to. This fix passes all testing. Kind regards, Evan ------------- Commit messages: - 8252883: AccessDeniedException caused by delayed file deletion on Windows Changes: https://git.openjdk.java.net/jdk/pull/2572/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8252883 Stats: 131 lines in 2 files changed: 124 ins; 5 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Mon, 15 Feb 2021 10:39:32 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
src/java.logging/share/classes/java/util/logging/FileHandler.java line 39:
37: import java.nio.channels.FileChannel; 38: import java.nio.channels.OverlappingFileLockException; 39: import java.nio.file.*;
We avoid using the wildcard with imports in the JDK sources. Could you revert that change? src/java.logging/share/classes/java/util/logging/FileHandler.java line 517:
515: continue; 516: } 517: else {
nit: can you put the closing brace and the else on the same line? src/java.logging/share/classes/java/util/logging/FileHandler.java line 512:
510: // Try again. If it doesn't work, then this will 511: // eventually ensure that we increment "unique" and 512: // use another file name.
I believe this would be more clear if the comment was put inside the if statement, just before continue. It might be good to add some words about what might be causing the AccessDeniedException as well... Maybe something like: } catch (AccessDeniedException ade) { // This can be either a temporary, or a more permanent issue. // The lock file might be still pending deletion from a previous run // (temporary), or the parent directory might not be accessible, etc... // If we can write to the current directory, and this is a regular file, // let's try again. if (Files.isRegularFile(lockFilePath, LinkOption.NOFOLLOW_LINKS) && isParentWritable(lockFilePath)) { // Try again. If it doesn't work, then this will // eventually ensure that we increment "unique" and // use another file name. continue; } else { throw ade; // no need to retry } ... test/jdk/java/util/logging/FileHandlerAccessTest.java line 47:
45: if (!(args.length == 2 || args.length == 1)) { 46: System.out.println("Usage error: expects java FileHandlerAccessTest [process/thread] <count>"); 47: System.exit(1);
We usually avoid `System.exit()` in tests. `return` should be enough. test/jdk/java/util/logging/FileHandlerAccessTest.java line 113:
111: } 112: } 113: }
Please add a newline at the end of the file. test/jdk/java/util/logging/FileHandlerAccessTest.java line 93:
91: bufferedReader = new BufferedReader(new InputStreamReader(childProcess.getInputStream())); 92: String line; 93: while((line = bufferedReader.readLine()) != null) {
space missing after `while` test/jdk/java/util/logging/FileHandlerAccessTest.java line 98:
96: childProcess.waitFor(); 97: } 98: catch(Exception e) {
space missing after `catch` test/jdk/java/util/logging/FileHandlerAccessTest.java line 106:
104: } 105: if (bufferedReader != null){ 106: try{
space missing after `try` test/jdk/java/util/logging/FileHandlerAccessTest.java line 109:
107: bufferedReader.close(); 108: } 109: catch(Exception ignored){}
space missing after `catch` test/jdk/java/util/logging/FileHandlerAccessTest.java line 101:
99: e.printStackTrace(); 100: } 101: finally {
I would prefer if `catch` and `finally` where on the same line than the preceding closing brace: try { ... } catch (...) { ... } finally { ... } test/jdk/java/util/logging/FileHandlerAccessTest.java line 75:
73: } 74: catch(Exception e) { 75: e.printStackTrace();
If you only print exceptions, when does the test fail? test/jdk/java/util/logging/FileHandlerAccessTest.java line 99:
97: } 98: catch(Exception e) { 99: e.printStackTrace();
Same remark here: should this make the test fail? ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Mon, 15 Feb 2021 12:55:46 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Doc cleanup, code formatting and throwing exceptions instead of catching
src/java.logging/share/classes/java/util/logging/FileHandler.java line 39:
37: import java.nio.channels.FileChannel; 38: import java.nio.channels.OverlappingFileLockException; 39: import java.nio.file.*;
We avoid using the wildcard with imports in the JDK sources. Could you revert that change?
Yes, reverted thanks
src/java.logging/share/classes/java/util/logging/FileHandler.java line 512:
510: // Try again. If it doesn't work, then this will 511: // eventually ensure that we increment "unique" and 512: // use another file name.
I believe this would be more clear if the comment was put inside the if statement, just before continue. It might be good to add some words about what might be causing the AccessDeniedException as well... Maybe something like:
} catch (AccessDeniedException ade) { // This can be either a temporary, or a more permanent issue. // The lock file might be still pending deletion from a previous run // (temporary), or the parent directory might not be accessible, etc... // If we can write to the current directory, and this is a regular file, // let's try again. if (Files.isRegularFile(lockFilePath, LinkOption.NOFOLLOW_LINKS) && isParentWritable(lockFilePath)) { // Try again. If it doesn't work, then this will // eventually ensure that we increment "unique" and // use another file name. continue; } else { throw ade; // no need to retry } ...
I agree. I've updated the comments
test/jdk/java/util/logging/FileHandlerAccessTest.java line 47:
45: if (!(args.length == 2 || args.length == 1)) { 46: System.out.println("Usage error: expects java FileHandlerAccessTest [process/thread] <count>"); 47: System.exit(1);
We usually avoid `System.exit()` in tests. `return` should be enough.
Changed to `return`
test/jdk/java/util/logging/FileHandlerAccessTest.java line 75:
73: } 74: catch(Exception e) { 75: e.printStackTrace();
If you only print exceptions, when does the test fail?
Thanks for catching this. I've updated the test to throw the exceptions rather than printing stack trace. This was a modified version of the test for debugging purposes. It's worth noting that it's not possible to write a test which can deterministically prove/ verify the behaviour (and related fix) of the parent bug as this involves the situation where multiple threads conflict on a Windows machine. The test attached is a best-effort and was tried around 40 times.
test/jdk/java/util/logging/FileHandlerAccessTest.java line 98:
96: childProcess.waitFor(); 97: } 98: catch(Exception e) {
space missing after `catch`
Fixed
test/jdk/java/util/logging/FileHandlerAccessTest.java line 99:
97: } 98: catch(Exception e) { 99: e.printStackTrace();
Same remark here: should this make the test fail?
Thanks Daniel, please see my above reply
test/jdk/java/util/logging/FileHandlerAccessTest.java line 101:
99: e.printStackTrace(); 100: } 101: finally {
I would prefer if `catch` and `finally` where on the same line than the preceding closing brace:
try { ... } catch (...) { ... } finally { ... }
Fixed
test/jdk/java/util/logging/FileHandlerAccessTest.java line 106:
104: } 105: if (bufferedReader != null){ 106: try{
space missing after `try`
Fixed
test/jdk/java/util/logging/FileHandlerAccessTest.java line 109:
107: bufferedReader.close(); 108: } 109: catch(Exception ignored){}
space missing after `catch`
Fixed
test/jdk/java/util/logging/FileHandlerAccessTest.java line 113:
111: } 112: } 113: }
Please add a newline at the end of the file.
Done ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: 8252883: Doc cleanup, code formatting and throwing exceptions instead of catching ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/045d725f..e755d323 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=00-01 Stats: 36 lines in 2 files changed: 13 ins; 5 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 16 Feb 2021 11:33:08 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Doc cleanup, code formatting and throwing exceptions instead of catching
Daniel's suggestions have been integrated to the patch ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 16 Feb 2021 11:37:05 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Doc cleanup, code formatting and throwing exceptions instead of catching
Thanks for taking on the changes. Still some comments... test/jdk/java/util/logging/FileHandlerAccessTest.java line 75:
73: handler.close(); 74: } catch (Exception e) { 75: throw new RuntimeException(e);
Will throwing an exception in a thread (if happening in the Thread created at line 60) cause jtreg to fail? Or will the exception simply be discarded? test/jdk/java/util/logging/FileHandlerAccessTest.java line 98:
96: childProcess.waitFor(); 97: } catch (Exception e) { 98: throw new RuntimeException(e);
Same holds there as well. Maybe you could make an experiment with a dummy test to see whether the exception will make the test fail. test/jdk/java/util/logging/FileHandlerAccessTest.java line 96:
94: System.out.println(name + "\t|" + line); 95: } 96: childProcess.waitFor();
Should you be checking the status returned by `waitFor` and fail the test if it's not `0`? ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 16 Feb 2021 11:33:08 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Doc cleanup, code formatting and throwing exceptions instead of catching
test/jdk/java/util/logging/FileHandlerAccessTest.java line 75:
73: handler.close(); 74: } catch (Exception e) { 75: throw new RuntimeException(e);
Will throwing an exception in a thread (if happening in the Thread created at line 60) cause jtreg to fail? Or will the exception simply be discarded?
The exception will in fact make the jtreg test fail. I verified this by creating a dummy test case and throwing an exception within a thread as you suggested.
test/jdk/java/util/logging/FileHandlerAccessTest.java line 98:
96: childProcess.waitFor(); 97: } catch (Exception e) { 98: throw new RuntimeException(e);
Same holds there as well. Maybe you could make an experiment with a dummy test to see whether the exception will make the test fail.
Please see above comment. Verified that an exception thrown within a thread causes test to fail ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 16 Feb 2021 11:48:05 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Doc cleanup, code formatting and throwing exceptions instead of catching
test/jdk/java/util/logging/FileHandlerAccessTest.java line 96:
94: System.out.println(name + "\t|" + line); 95: } 96: childProcess.waitFor();
Should you be checking the status returned by `waitFor` and fail the test if it's not `0`?
I'll modify the code to add this behaviour. Thanks for the suggestion :) ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: 8252883: Throw exception if exit code of child process is non-zero ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/e755d323..9472f73c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=01-02 Stats: 9 lines in 1 file changed: 4 ins; 1 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 16 Feb 2021 21:15:58 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Throw exception if exit code of child process is non-zero
test/jdk/java/util/logging/FileHandlerAccessTest.java line 9:
7: * published by the Free Software Foundation. Oracle designates this 8: * particular file as subject to the "Classpath" exception as provided 9: * by Oracle in the LICENSE file that accompanied this code.
Tests do not need, and should not have, the CPE. ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Wed, 17 Feb 2021 07:37:42 GMT, David Holmes <dholmes@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Throw exception if exit code of child process is non-zero
test/jdk/java/util/logging/FileHandlerAccessTest.java line 9:
7: * published by the Free Software Foundation. Oracle designates this 8: * particular file as subject to the "Classpath" exception as provided 9: * by Oracle in the LICENSE file that accompanied this code.
Tests do not need, and should not have, the CPE.
Fixed, thanks ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: 8252883: Remove ClassPathException copyright statement ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/9472f73c..26c56c47 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=02-03 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Wed, 17 Feb 2021 11:50:05 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Remove ClassPathException copyright statement
test/jdk/java/util/logging/FileHandlerAccessTest.java line 45:
43: if (!(args.length == 2 || args.length == 1)) { 44: System.out.println("Usage error: expects java FileHandlerAccessTest [process/thread] <count>"); 45: return;
Ah - sorry - since this is a test, instead of return you should probably throw an exception - e.g.: throw new IllegalArgumentException("Usage error: expects java FileHandlerAccessTest [process/thread] <count>"); test/jdk/java/util/logging/FileHandlerAccessTest.java line 47:
45: return; 46: } 47: else if (args.length == 2) {
nit: `} else if (...) {` ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Wed, 17 Feb 2021 12:24:46 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Remove ClassPathException copyright statement
test/jdk/java/util/logging/FileHandlerAccessTest.java line 45:
43: if (!(args.length == 2 || args.length == 1)) { 44: System.out.println("Usage error: expects java FileHandlerAccessTest [process/thread] <count>"); 45: return;
Ah - sorry - since this is a test, instead of return you should probably throw an exception - e.g.: throw new IllegalArgumentException("Usage error: expects java FileHandlerAccessTest [process/thread] <count>");
Done! Thanks Daniel
test/jdk/java/util/logging/FileHandlerAccessTest.java line 47:
45: return; 46: } 47: else if (args.length == 2) {
nit: `} else if (...) {`
Fixed ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: 8252883: Added IllegalArgumentException for incorrect usage ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/26c56c47..b5259dcb Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=03-04 Stats: 6 lines in 1 file changed: 0 ins; 3 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Wed, 17 Feb 2021 14:47:03 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Added IllegalArgumentException for incorrect usage
LGTM ------------- Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2572
On Thu, 18 Feb 2021 10:41:17 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Added IllegalArgumentException for incorrect usage
LGTM
I'm seeing some intermittent test failures with the new test (after merging in latest master changes). Can you retest and have a look: ----------System.out:(25/1343)---------- Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Thread-2 |Error: Could not find or load main class FileHandlerAccessTest Thread-2 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-6 |Error: Could not find or load main class FileHandlerAccessTest Thread-6 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-1 |Error: Could not find or load main class FileHandlerAccessTest Thread-1 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Testing with arguments: type=process, count=20 ----------System.err:(14/1024)---------- java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. ----------rerun:(39/6191)*---------- ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Fri, 19 Feb 2021 14:06:23 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
LGTM
I'm seeing some intermittent test failures with the new test (after merging in latest master changes). Can you retest and have a look:
----------System.out:(25/1343)---------- Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Thread-2 |Error: Could not find or load main class FileHandlerAccessTest Thread-2 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-6 |Error: Could not find or load main class FileHandlerAccessTest Thread-6 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-1 |Error: Could not find or load main class FileHandlerAccessTest Thread-1 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Testing with arguments: type=process, count=20 ----------System.err:(14/1024)---------- java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. ----------rerun:(39/6191)*----------
Hi, I've removed the problematic "process" handling logic and have stripped the test back to only use threads. The problem was reproducible (intermittently on my Windows machine) using this smaller test and should make the test more stable. ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Mon, 22 Feb 2021 09:46:56 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
I'm seeing some intermittent test failures with the new test (after merging in latest master changes). Can you retest and have a look:
----------System.out:(25/1343)---------- Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Testing with arguments: type=process, count=20 Thread-2 |Error: Could not find or load main class FileHandlerAccessTest Thread-2 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-6 |Error: Could not find or load main class FileHandlerAccessTest Thread-6 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Thread-1 |Error: Could not find or load main class FileHandlerAccessTest Thread-1 |Caused by: java.lang.ClassNotFoundException: FileHandlerAccessTest Testing with arguments: type=process, count=20 ----------System.err:(14/1024)---------- java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:96) at java.base/java.lang.Thread.run(Thread.java:831) Caused by: java.lang.RuntimeException: An error occured in the child process. at FileHandlerAccessTest.accessProcess(FileHandlerAccessTest.java:93) ... 1 more STATUS:Failed.`main' threw exception: java.lang.RuntimeException: java.lang.RuntimeException: An error occured in the child process. ----------rerun:(39/6191)*----------
Hi,
I've removed the problematic "process" handling logic and have stripped the test back to only use threads.
The problem was reproducible (intermittently on my Windows machine) using this smaller test and should make the test more stable.
Does the new version of the test still occasionally catches the issue and fails if the fix is not present? ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 10:16:26 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Hi,
I've removed the problematic "process" handling logic and have stripped the test back to only use threads.
The problem was reproducible (intermittently on my Windows machine) using this smaller test and should make the test more stable.
Does the new version of the test still occasionally catches the issue and fails if the fix is not present?
Hi, locally this test reproduces the issue on my Windows machine. I believe our internal testing Windows boxes are too powerful and handle the threading too efficiently to reproduce this error. I can reproduce locally approx 10% of the time, but after over 100 runs I cannot reproduce as part of our internal testing. As this is not verifiable in a completely deterministic way, I believe having the test as a stable, passing test is a more appropriate approach instead of selecting this as no-reg hard. It adds more test coverage to the code also. ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 12:10:30 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Does the new version of the test still occasionally catches the issue and fails if the fix is not present?
Hi, locally this test reproduces the issue on my Windows machine.
I believe our internal testing Windows boxes are too powerful and handle the threading too efficiently to reproduce this error.
I can reproduce locally approx 10% of the time, but after over 100 runs I cannot reproduce as part of our internal testing.
As this is not verifiable in a completely deterministic way, I believe having the test as a stable, passing test is a more appropriate approach instead of selecting this as no-reg hard.
It adds more test coverage to the code also.
Reproducing locally with the test is fine. That's enough for me. ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: 8252883: Stripped back FileHandlerAccessTest to only use threads ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/b5259dcb..1bb8837e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=04-05 Stats: 58 lines in 1 file changed: 0 ins; 53 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Mon, 22 Feb 2021 09:50:06 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Stripped back FileHandlerAccessTest to only use threads
test/jdk/java/util/logging/FileHandlerAccessTest.java line 2:
1: /* 2: * Copyright (c) 2000, 2021, Oracle and/or its affiliates. All rights reserved.
This seems like a new test - should it only have: * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. ``` ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 10:14:07 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
8252883: Stripped back FileHandlerAccessTest to only use threads
test/jdk/java/util/logging/FileHandlerAccessTest.java line 2:
1: /* 2: * Copyright (c) 2000, 2021, Oracle and/or its affiliates. All rights reserved.
This seems like a new test - should it only have:
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. ```
Thanks, will update ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision: Fixed copyright year in FileHandlerAccessTest.java ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2572/files - new: https://git.openjdk.java.net/jdk/pull/2572/files/1bb8837e..c3b1d81d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2572&range=05-06 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2572.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2572/head:pull/2572 PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 12:15:08 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
Fixed copyright year in FileHandlerAccessTest.java
Marked as reviewed by dfuchs (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 13:08:29 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
Fixed copyright year in FileHandlerAccessTest.java
Marked as reviewed by dfuchs (Reviewer).
Thanks for the feedback Daniel! As I've already posted the integrate command, I believe all this needs now is a sponsor :) ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 14:00:16 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Marked as reviewed by dfuchs (Reviewer).
Thanks for the feedback Daniel! As I've already posted the integrate command, I believe all this needs now is a sponsor :)
Oops, never mind I've to re-issue the command ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
On Tue, 23 Feb 2021 12:15:08 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
Evan Whelan has updated the pull request incrementally with one additional commit since the last revision:
Fixed copyright year in FileHandlerAccessTest.java
Looks OK to me, although at my local Windows 10 box test took more than 100 iteration before it failed. ------------- Marked as reviewed by vyommani@github.com (no known OpenJDK username). PR: https://git.openjdk.java.net/jdk/pull/2572
On Mon, 15 Feb 2021 10:39:32 GMT, Evan Whelan <ewhelan@openjdk.org> wrote:
Hi,
Please review this fix for JDK-8252883. This handles the case when an AccessDeniedException is being thrown on Windows, due to a delay in deleting the lock file it is trying to write to.
This fix passes all testing.
Kind regards, Evan
This pull request has now been integrated. Changeset: ebdc80ea Author: Evan Whelan <ewhelan@openjdk.org> Committer: Daniel Fuchs <dfuchs@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/ebdc80ea Stats: 73 lines in 2 files changed: 72 ins; 0 del; 1 mod 8252883: AccessDeniedException caused by delayed file deletion on Windows Reviewed-by: dfuchs ------------- PR: https://git.openjdk.java.net/jdk/pull/2572
participants (4)
-
Daniel Fuchs
-
David Holmes
-
Evan Whelan
-
Vyom Mani Tewari