Hi Sergey, Thanks for the clarification. I have updated the test case to use Files.delete(). Please find updated webrev for review: http://cr.openjdk.java.net/~jdv/4924727/webrev.02/
Regards, Jay -----Original Message----- From: Sergey Bylokhov Sent: Monday, August 29, 2016 8:52 PM To: Jayathirth D V Cc: 2d-dev Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-4924727 : reader.abort() method does not work when called inside imageStarted for PNG On 29.08.16 18:07, Jayathirth D V wrote: > Hi Sergey, > > I am not getting the usage of Files.delete() from its specification. Can you > please elaborate what special case it will handle in my test case? > I am creating temporary file separately for all the readers and deleting them. Files.delete() will throw an exception if the file cannot be deleted, and File.delete() will return false in such case. Also I am closing the ImageInputStream associated after read operation. But plugin itself can leak some streams and lock a temporary file, so Files.delete() will catch this. > -----Original Message----- > From: Sergey Bylokhov > Sent: Monday, August 29, 2016 8:25 PM > To: Jayathirth D V; Philip Race > Cc: Prasanta Sadhukhan; 2d-dev > Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-4924727 : reader.abort() method > does not work when called inside imageStarted for PNG > > Hi, Jay. > Please delete the temporary file via Files.delete(), which will throw an > exception if the file is locked by some reader. > > On 29.08.16 11:42, Jayathirth D V wrote: >> Hi Phil & Sergey, >> >> Thanks for your inputs. >> >> I have verified reader.abort() request for IIOReadProgressListener for all >> available plugins. >> >> Apart from PNG although all readers were able to abort read when we call >> reader.abort() from IIOReadProgressListener callbacks, they were not calling >> processReadAborted() right after IIOReadProgressListener callbacks. So I >> have made changes for the same. >> >> And in some readers before every read call they were not calling >> clearAbortRequest(), which is important because if we use same reader for >> another read() call it will be invalid unless we clear previous abort >> request. >> >> In case of JPEG since we are using native IJG library we need to update >> abortFlag present in imageioJPEG.c before every call as we are doing for >> other readers using clearAbortRequest(). >> >> Since this has native and make changes I have verified changes through >> JPRT also which is successfully building on all platforms >> (http://scaaa637.us.oracle.com//archive/2016/08/2016-08-29-065104.jay. >> client_commit//JobStatus.txt ) >> >> Please find updated webrev for review: >> http://cr.openjdk.java.net/~jdv/4924727/webrev.01/ >> >> I noticed that in case on WBMP I was not getting ImageReader object to call >> setInput() in test case to verify the behavior of reader.abort(). So I have >> created separate bug for the same >> (https://bugs.openjdk.java.net/browse/JDK-8164930 ). And in case of WBMP we >> already have clearAbortRequest() call and also we are returning from >> IIOReadProgressListener callbacks properly, only thing here is we are not >> returning right after callbacks as we have updated other plugins. >> >> I want to verify writer plugins in separate bug as we already have lot of >> changes in this bug. So I have created >> https://bugs.openjdk.java.net/browse/JDK-8164931 and will be working on this >> bug. >> >> Thanks, >> Jay >> >> -----Original Message----- >> From: Phil Race >> Sent: Thursday, August 18, 2016 1:42 AM >> To: Sergey Bylokhov >> Cc: Jayathirth D V; Prasanta Sadhukhan; 2d-dev >> Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-4924727 : reader.abort() >> method does not work when called inside imageStarted for PNG >> >> I think we can >> - get all plugins,and for each >> - write a file in that format >> - read it back and apply the test >> >> It is also worth verifying that the writer abort checks are in sync with the >> reader aborts, ie happen at such equivalent points as might exist. >> >> -phil. >> >> On 08/15/2016 11:30 AM, Sergey Bylokhov wrote: >>> Is it possible to unify the test for all our plugins? I assume they >>> should work in the same way. I am not sure but probably the image can >>> be generated at runtime? >>> >>> On 11.08.16 21:59, Jayathirth D V wrote: >>>> Hi, >>>> >>>> >>>> >>>> Please review the following fix in JDK9 at your convenience: >>>> >>>> >>>> >>>> Bug : https://bugs.openjdk.java.net/browse/JDK-4924727 >>>> >>>> >>>> >>>> Webrev : http://cr.openjdk.java.net/~jdv/4924727/webrev.00/ >>>> >>>> >>>> >>>> Issue : When we issue ImageReader.abort() in >>>> IIOReadProgressListener.imageStarted(), reading is not aborted and >>>> it is continued. >>>> >>>> >>>> >>>> Root cause : After IIOReadProgressListener.imageStarted() call in >>>> PNGImageReader.java when we enter decodeImage() we call >>>> clearAbortRequest() which will clear the abort request from >>>> IIOReadProgressListener.imageStarted(). >>>> >>>> >>>> >>>> Solution : clearAbortRequest() documentation mentions that it should >>>> be called before reading of image starts, so it should be called >>>> before IIOReadProgressListener.imageStarted()(In PNGImageReader.java >>>> it is >>>> processImageStarted(0) in readImage()). So moved clearAbortRequest() >>>> call from decodeImage() to readImage(). Also we should call >>>> abortRequested() in PNGImageReader.java at places mapping to >>>> IIOReadProgressListener and not randomly at end of functions or at >>>> places related to IIOReadUpdateListener, updated the code for the same. >>>> >>>> >>>> >>>> >>>> >>>> Observation not related to this issue : We don't have call similar >>>> to >>>> IIOReadProgressListener.readAborted() in IIOReadUpdateListener, but >>>> user can call ImageReader.abort() from IIOReadUpdateListener methods. >>>> Is there a need to add similar method in IIOReadUpdateListener? Any >>>> inputs on this also would be helpful. >>>> >>>> >>>> >>>> Thanks, >>>> >>>> Jay >>>> >>> >>> >> > > > -- > Best regards, Sergey. > -- Best regards, Sergey.
