Thanks for the review Sergey.
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, August 31, 2016 6:08 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 31.08.16 14:48, Jayathirth D V wrote:
> Hi Sergey,
>
> In case of JPEG whole read process is under a ThreadLock.
> public BufferedImage read(int imageIndex, ImageReadParam param)
> throws IOException {
> setThreadLock();
> try {
> cbLock.check();
> try {
> readInternal(imageIndex, param, false);
Then the fix looks fine.
>
> By others processXXX() do you mean processXXX() in other plugins or
> processXXX() in case of JPEG?
> Please clarify.
I meant only the code related to jpeg.
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Wednesday, August 31, 2016 4:22 PM
> To: Jayathirth D V; Philip Race
> Cc: 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-4924727 : reader.abort()
> method does not work when called inside imageStarted for PNG
>
> I have only one question: should we call the new clearNativeReadAbortFlag(and
> probably the others processXXX()) under the ThreadLock?
>
> On 31.08.16 13:07, Jayathirth D V wrote:
>> 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.
>
--
Best regards, Sergey.
