[OpenJDK 2D-Dev] [10]JDK-8183341:Better cleanup for javax/imageio/AllowSearch.java

Prahalad Kumar Narayanan prahalad.kumar.narayanan at oracle.com
Wed Jul 12 05:41:39 UTC 2017


Hello Sashi

As Sergey suggested, the deleting the file in the finally block resolves the issues.
The change looks good.

Thanks
Have a good day

Prahalad N.

----------------------------------------------------
From: Shashidhara Veerabhadraiah 
Sent: Wednesday, July 12, 2017 11:03 AM
To: Sergey Bylokhov; Prahalad Kumar Narayanan
Cc: Prasanta Sadhukhan; 2d-dev at openjdk.java.net; Philip Race
Subject: RE: [OpenJDK 2D-Dev] [10]JDK-8183341:Better cleanup for javax/imageio/AllowSearch.java

Sergey, Thank you for the excellent suggestion. I have updated the Webrev accordingly.

Prahalad, With the use of Sergey’s suggestion of adding an finally block solves the problem of not deleting files on a fail case. 
      . Was the bug reproducible at your end ?
Yes, it was reproducible. We can see the temp files hanging at %temp% folder.
      . If yes, was it reproducible when the test succeeded /or when the test failed ?
Yes, it was reproducible on success for sure. The fail case was not tested as that would require me to do an undo of an earlier changeset change. But I think the output would remain the same as the pass case.
      . Is there a difference in the VM's termination based on the outcome of the test case.
Not sure since the fail case was not tested. But per the code, it is sure that it will have differences. The finally block should fix this problem.
      . How many such test cases exist that need similar clean up ?
            . grep on 'deleteOnExit' within jdk/test/javax/imageio/ gives me atleast 10 instances.
            . grep on 'File.createTempFile' within same directory gives me 29 instances.
Am not sure what to do with respect to them. Should I raise new bugs if there is problem with them(if not on the bug db) and fix it?

Updated Webrev:
http://cr.openjdk.java.net/~pkbalakr/shashi/8183341/webrev_01/

Thanks and regards,
Shashi

From: Sergey Bylokhov 
Sent: Wednesday, July 12, 2017 12:13 AM
To: Shashidhara Veerabhadraiah <shashidhara.veerabhadraiah at oracle.com>
Cc: Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com>; 2d-dev at openjdk.java.net; Philip Race <philip.race at oracle.com>
Subject: Re: [OpenJDK 2D-Dev] [10]JDK-8183341:Better cleanup for javax/imageio/AllowSearch.java

Hi Shashi.
I suggest to add a finally to the try block in the test() method and delete the file, close the stream and close the reader there.

----- shashidhara.veerabhadraiah at oracle.com wrote: 
> 
Hi All,
Please review a fix for a test bug which was not cleaning up the temporary test files that it used to create.
The issue with this test was that the test used to create temporary files but not deleting them.
The updated test file does the deletion of the temporary files that the test is creating.
Bug:
<https://bugs.openjdk.java.net/browse/JDK-8183341>
Webrev:
<http://cr.openjdk.java.net/~pkbalakr/shashi/8183341/webrev_00/>
Note: The user had requested to create the temporary files in user.dir. But I think it is good to create the temporary files in the system temp directory and use it for testing and later delete the same. Besides if required the user has the choice to change the temp directory to the directory they wish for.
Thanks and regards,
Shashi


More information about the 2d-dev mailing list