RFR: JDK-8158066: SourceDebugExtensionTest fails to rename file

[David Holmes]

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
>>>>>>
>>>>
>>
>