On 1/17/19 6:06 AM, gary.ad...@oracle.com wrote:
Updated webrev:
Webrev: http://cr.openjdk.java.net/~gadams/8158066/webrev.01/
test/hotspot/jtreg/vmTestbase/nsk/share/jdi/sde/InstallSDE.java
No comments.
test/jdk/com/sun/jdi/sde/InstallSDE.java
No comments.
test/jdk/ProblemList.txt
No comments.
Thumbs up!
Dan
On 1/17/19 12:44 AM, David Holmes wrote:
On 17/01/2019 1:48 am, Gary Adams wrote:
Looking through the test history for the vm/mlvm version of
InstallSDE.java there has never
been a reported problem with the renameTo operation. I'm inclined to
not apply the change
made to the com/sun/jdi/sde test until the failure is observed.
There is much greater use of InstallSDE than just the vm/mlvm test
that I mentioned earlier. InstallSDE is also used by:
./hotspot/jtreg/vmTestbase/nsk/share/jdi/sde/SDEDebugger.java
which in turn is used by numerous nsk/jdi tests.
It would be prudent IMHO to fix both versions. It would be extremely
surprising (and raise doubt about the actual problem) if the jdk
tests fail but the nsk tests never do.
David
-----
I'm satisfied with the testing at this point and could use another
reviewer before
pushing.
On 1/11/19, 6:14 PM, David Holmes wrote:
Hi Gary,
As I mentioned earlier there are two versions of InstallSDE.java in
the tests, so both should be updated.
Your strategy to avoid the windows delete issue seems reasonable.
David
On 12/01/2019 4:53 am, Gary Adams wrote:
Here's a webrev for review purposes.
No failures after ~1000 testruns.
Webrev: http://cr.openjdk.java.net/~gadams/8158066/webrev.00/
Issue: https://bugs.openjdk.java.net/browse/JDK-8158066
On 1/11/19, 11:01 AM, Gary Adams wrote:
I've been reading recently that Windows delete file operations
can be delayed, if there are open handles against the file.
Others have reported virus checkers or other indexing
programs may hold a handle on a file preventing the
deletion being completed.
The basic logic in InstallSDE is
A + B = C
delete A
rename C to A
I think we can work around the issue with
rename A to A1
A1 + B = C
delete A1
rename C to A
diff --git a/test/jdk/com/sun/jdi/sde/InstallSDE.java
b/test/jdk/com/sun/jdi/sde/InstallSDE.java
--- a/test/jdk/com/sun/jdi/sde/InstallSDE.java
+++ b/test/jdk/com/sun/jdi/sde/InstallSDE.java
@@ -31,9 +31,17 @@
}
static void install(File inOutClassFile, File attrFile)
throws IOException {
- File tmpFile = new File(inOutClassFile.getPath() + "tmp");
- new InstallSDE(inOutClassFile, attrFile, tmpFile);
- if (!inOutClassFile.delete()) {
+ File tmpFile = new File(inOutClassFile.getPath() +
"tmp-out");
+ File tmpInOutClassFile = new
File(inOutClassFile.getPath() + "tmp-in");
+
+ // Workaround delayed file deletes on Windows using a
tmp file name
+ if (!inOutClassFile.renameTo(tmpInOutClassFile)) {
+ throw new IOException("tmp copy of inOutClassFile
failed");
+ }
+
+ new InstallSDE(tmpInOutClassFile, attrFile, tmpFile);
+
+ if (!tmpInOutClassFile.delete()) {
throw new IOException("inOutClassFile.delete()
failed");
}
if (!tmpFile.renameTo(inOutClassFile)) {
Testing in progress ...
On 1/11/19, 7:40 AM, David Holmes wrote:
Hi Gary,
On 11/01/2019 9:22 pm, gary.ad...@oracle.com wrote:
After ~1000 testruns I hit a failure in MangleTest and a jtreg
agent communication issue with SourceDebugExtension.
https://java.se.oracle.com:10065/mdash/builds/2019-01-10-1159535.gary.adams.jdk-jdb/results?search=status%3Afailed%20AND%20-state%3Ainvalid
*java.io.IOException: tmpFile.renameTo(inOutClassFile) failed*
at InstallSDE.install(InstallSDE.java:40)
at MangleTest.testSetUp(MangleTest.java:38)
at MangleTest.main(MangleTest.java:31)
You might want to add some code in:
./java.base/windows/native/libjava/WinNTFileSystem_md.c:Java_java_io_WinNTFileSystem_rename0
that checks the result of _wrename to see what error is
occurring. I suspect it will be EACCES which won't be that helpful:
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/rename-wrename?view=vs-2017
but it would be good to confirm.
result: Error. Agent communication error:
java.io.IOException:*Agent: unexpected op: 71;* check console
log for any additional details
I added some additional print outs for the rename issue and
have run ~2000 additional
testruns with no test failures repeated, yet. But there were 2
mach5 reported errors:
TimeoutException in CLEANUP.
That's a mach5 issue.
David
I'll do some more investigation with making the rename
operation more robust
before removing SourceDebugExtensionTest from the problem list.
On 1/10/19 8:28 PM, David Holmes wrote:
Hi Gary,
On 10/01/2019 11:02 pm, gary.ad...@oracle.com wrote:
SourceDebugExtensionTest was placed on the ProblemList
because of filesystem rename issues on Windows 2012 test
systems.
The test does not appear to fail on the current mach5 test
systems.
There was some question concerning symbolic links that might
have
been problematic at that time. Additional testing in progress...
Also note that there are two sets of SDE related tests and two
versions of the InstallSDE.java file that both use renameTo.
In nsk the testcase seems to be:
./hotspot/jtreg/vmTestbase/vm/mlvm/share/StratumClassesBuilder.java
and AFAICS it is not excluded. So removing the exclusion seems
reasonable to me. It was most like a concurrency issue with
the virus scanner.
Thanks,
David
Issue: https://bugs.openjdk.java.net/browse/JDK-8158066
diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -838,8 +838,6 @@
com/sun/jdi/RepStep.java 8043571 generic-all
-com/sun/jdi/sde/SourceDebugExtensionTest.java 8158066
windows-all
-
com/sun/jdi/NashornPopFrameTest.java 8187143 generic-all