RFR: JDK-8158066: SourceDebugExtensionTest fails to rename file
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Jan 22 20:32:10 UTC 2019
Hi Gary,
It looks Okay in general.
But I have a small concern/question about the line:
65 if (!inOutClassFile.renameTo(tmpInOutClassFile)) { What if the
tmpInOutClassFile does exist in the system?
Is it possible there can be some leftover from previous runs?
Thanks,
Serguei
On 1/17/19 3:06 AM, gary.adams at oracle.com wrote:
> Updated webrev:
>
> Webrev: http://cr.openjdk.java.net/~gadams/8158066/webrev.01/
>
> 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.adams at 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.adams at 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
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190122/a1dc44f0/attachment-0001.html>
More information about the serviceability-dev
mailing list