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

By others processXXX() do you mean processXXX() in other plugins or 
processXXX() in case of JPEG?
Please clarify.

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.

Reply via email to