[OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Jayathirth D V
Hi, Please review the following fix in JDK9 : Bug : https://bugs.openjdk.java.net/browse/JDK-7059970 Webrev : http://cr.openjdk.java.net/~jdv/7059970/webrev.00/ Root cause : Test case ITXtTest.java is not deleting the file it is creating(test.png). Also it is not closing ImageInputS

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Brian Burkhalter
Why not simply invoke deleteOnExit() on the file instance immediately after it is created and use try-with-resources blocks for the Image{Input,Output}Stream instances? Brian On Jul 7, 2016, at 4:38 AM, Jayathirth D V wrote: > Please review the following fix in JDK9 : > > Bug : https://bugs

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Phil Race
you need to add the new bug to @bug .. 197 imageWriter.write(new IIOImage(src, null, m)); 198 imageOutputStream.close(); <<< 199 System.out.println("Writing done."); 200 } catch (Throwable e) { 201 if (imageOutputStream

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Phil Race
I suppose deleteOnExit might be OK but jtreg re-uses the VM so it will not get deleted until much later - after all N thousand tests have been run .. assuming no other test crashes the VM before then. -phil. On 07/07/2016 10:41 AM, Brian Burkhalter wrote: Why not simply invoke deleteOnExit() o

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-07 Thread Brian Burkhalter
Good point - probably better to make it explicit then. The try-with-resources should still be cleaner for closing the streams however. Brian On Jul 7, 2016, at 1:26 PM, Phil Race wrote: > I suppose deleteOnExit might be OK but jtreg re-uses the VM > so it will not get deleted until much later

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-08 Thread Jayathirth D V
2:17 AM To: Phil Race Cc: Jayathirth D V; 2d-dev Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file Good point - probably better to make it explicit then. The try-with-resources should still be cleaner for closing the s

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-08 Thread Brian Burkhalter
Hi Jay, Sorry to be picky here but in doTest() could you not instead have try { writeTo(file, src); ITXtTest dst = readFrom(file); if (dst == null || !dst.equals(src)) { throw new RuntimeException("Test failed."); } }

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-12 Thread Jayathirth D V
e; 2d-dev Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file Hi Jay, Sorry to be picky here but in doTest() could you not instead have try { writeTo(file, src); ITXtTes

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-12 Thread Brian Burkhalter
Hi Jay, Looks fine to me. Thanks, Brian On Jul 12, 2016, at 5:03 AM, Jayathirth D V wrote: > That’s very good thing to do as it will remove redundant f.delete(). > I have updated the webrev for review : > http://cr.openjdk.java.net/~jdv/7059970/webrev.02/

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Prasanta Sadhukhan
1:35 PM *To:* Jayathirth D V *Cc:* Philip Race; 2d-dev *Subject:* Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file Hi Jay, Sorry to be picky here but in doTest() could you not instead have try { writeT

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Jayathirth D V
To: Jayathirth D V Cc: 2d-dev Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file Hi Jay, Why do we need to catch and throw RuntimeExcpn? try { } catch (RuntimeException e) { throw e; } Can't we just do

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Prasanta Sadhukhan
9970/webrev.02/> Thanks, Jay *From:*Brian Burkhalter *Sent:* Friday, July 08, 2016 11:35 PM *To:* Jayathirth D V *Cc:* Philip Race; 2d-dev *Subject:* Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not c

Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file

2016-07-13 Thread Jayathirth D V
nt: Friday, July 08, 2016 11:35 PM To: Jayathirth D V Cc: Philip Race; 2d-dev Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7059970 : Test case: javax/imageio/plugins/png/ITXtTest.java is not closing a file Hi Jay, Sorry to be picky here but in doTest() could you not instead have