[10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

[10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Everyone

Good day to you.

Kindly review a fix for the bug
    Bug ID: JDK-8188083
    Description: Null Pointer Exception in java.awt.image.FilteredImageSource.startProduction

Root Cause
    . FilteredImageSource implements ImageProducer interface
    . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
    . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.

Exception occurrence
    . The submitter has mentioned that there is no test-case to reproduce to this issue.
    . Luckily I was able to observe this issue with a "crude" test code
          . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
          . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
          . Hence the test code isn't added to the webrev.

Details on the Fix:
    . The concerned methods have been "synchronized" in the fix.
    . No new regression failures were observed with this change.

Kindly review the change and provide your feedback.
In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.

Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/

Thank you for your time in review
Have a good day

Prahalad N.





Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Jayathirth D v
Hi Prahalad,

Changes are fine.

Thanks,
Jay

-----Original Message-----
From: Prahalad Kumar Narayanan
Sent: Thursday, October 26, 2017 9:37 AM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Hello Everyone

Good day to you.

Kindly review a fix for the bug
    Bug ID: JDK-8188083
    Description: Null Pointer Exception in java.awt.image.FilteredImageSource.startProduction

Root Cause
    . FilteredImageSource implements ImageProducer interface
    . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
    . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.

Exception occurrence
    . The submitter has mentioned that there is no test-case to reproduce to this issue.
    . Luckily I was able to observe this issue with a "crude" test code
          . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
          . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
          . Hence the test code isn't added to the webrev.

Details on the Fix:
    . The concerned methods have been "synchronized" in the fix.
    . No new regression failures were observed with this change.

Kindly review the change and provide your feedback.
In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.

Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/

Thank you for your time in review
Have a good day

Prahalad N.





Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
In reply to this post by Prahalad kumar Narayanan
Hi, Prahalad.
Can you please add a stack trace which include a line numbers to the bug
report? Currently it is unclear in what line an exception is occurred.

On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:

> Hello Everyone
>
> Good day to you.
>
> Kindly review a fix for the bug
>      Bug ID: JDK-8188083
>      Description: Null Pointer Exception in java.awt.image.FilteredImageSource.startProduction
>
> Root Cause
>      . FilteredImageSource implements ImageProducer interface
>      . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>      . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>
> Exception occurrence
>      . The submitter has mentioned that there is no test-case to reproduce to this issue.
>      . Luckily I was able to observe this issue with a "crude" test code
>            . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>            . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>            . Hence the test code isn't added to the webrev.
>
> Details on the Fix:
>      . The concerned methods have been "synchronized" in the fix.
>      . No new regression failures were observed with this change.
>
> Kindly review the change and provide your feedback.
> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>
> Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> Thank you for your time in review
> Have a good day
>
> Prahalad N.
>
>
>
>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Sergey

Thank you for the suggestion.

I 've updated JBS with below information.
   1 . Stack trace information as provided by submitter-
        java.lang.NullPointerException
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
            at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
            at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
    2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
            . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.

Thank you
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov
Sent: Thursday, October 26, 2017 12:05 PM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Hi, Prahalad.
Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.

On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:

> Hello Everyone
>
> Good day to you.
>
> Kindly review a fix for the bug
>      Bug ID: JDK-8188083
>      Description: Null Pointer Exception in
> java.awt.image.FilteredImageSource.startProduction
>
> Root Cause
>      . FilteredImageSource implements ImageProducer interface
>      . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>      . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>
> Exception occurrence
>      . The submitter has mentioned that there is no test-case to reproduce to this issue.
>      . Luckily I was able to observe this issue with a "crude" test code
>            . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>            . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>            . Hence the test code isn't added to the webrev.
>
> Details on the Fix:
>      . The concerned methods have been "synchronized" in the fix.
>      . No new regression failures were observed with this change.
>
> Kindly review the change and provide your feedback.
> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>
> Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> Thank you for your time in review
> Have a good day
>
> Prahalad N.
>
>
>
>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Phil Race
The screenshot shows you directly calling this method in your test which
the documentation says you are not supposed to do.
So I am not able to be 100% sure that the test you have re-creates what
the submitter saw .. in his stack trace you have below it seems to be valid.

But to have a NPE at line 181 is odd.
 181             proxies.put(ic, imgf);

What is null ? proxies was just initialised in this same method so
I don't see how it can be null unless something elsewhere is
resetting it to null.

The only place I see that is removeConsumer

 138     public synchronized void removeConsumer(ImageConsumer ic) {
 139         if (proxies != null) {
 140             ImageFilter imgf =  proxies.get(ic);
 141             if (imgf != null) {
 142                 src.removeConsumer(imgf);
 143                 proxies.remove(ic);
 144                 if (proxies.isEmpty()) {
 145                     proxies = null;
 146                 }
 147             }
 148         }
 149     }

Now that method is synchronized .. OK so I think it might make sense
to mark the additional methods synchronized too but then I automatically
worry about introducing a deadlock.

I can't say for sure that you have done so however and likely there
are no nested locks here so it is probably OK but please run as many
tests as you can find to try to ensure that.

Adding or removing sychronized is binary compatible but since
these are public API methods you should still do a CSR
before pushing if this ends up being the approved fix.

-phil.



On 10/25/17, 11:58 PM, Prahalad Kumar Narayanan wrote:
Hello Sergey

Thank you for the suggestion.

I 've updated JBS with below information.
   1 . Stack trace information as provided by submitter- 
        java.lang.NullPointerException 
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181) 
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183) 
            at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732) 
            at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221) 
    2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181. 
            . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.

Thank you
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov 
Sent: Thursday, October 26, 2017 12:05 PM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Hi, Prahalad.
Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.

On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
Hello Everyone

Good day to you.

Kindly review a fix for the bug
     Bug ID: JDK-8188083
     Description: Null Pointer Exception in 
java.awt.image.FilteredImageSource.startProduction

Root Cause
     . FilteredImageSource implements ImageProducer interface
     . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
     . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.

Exception occurrence
     . The submitter has mentioned that there is no test-case to reproduce to this issue.
     . Luckily I was able to observe this issue with a "crude" test code
           . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
           . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
           . Hence the test code isn't added to the webrev.

Details on the Fix:
     . The concerned methods have been "synchronized" in the fix.
     . No new regression failures were observed with this change.

Kindly review the change and provide your feedback.
In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.

Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/

Thank you for your time in review
Have a good day

Prahalad N.






--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
In reply to this post by Prahalad kumar Narayanan
One more note:
I guess it is possible to create a stable test like below which can
reproduce the bug:

import java.awt.image.ColorModel;
import java.awt.image.FilteredImageSource;
import java.awt.image.ImageConsumer;
import java.awt.image.ImageFilter;
import java.awt.image.ImageProducer;
import java.util.Hashtable;

public final class Test {

     private volatile static Throwable fail;

     public static void main(final String[] args) throws
InterruptedException {
         final ImageConsumer ic = new ImageConsumer() {
             @Override
             public void setDimensions(int i, int i1) {
             }

             @Override
             public void setProperties(Hashtable<?, ?> hashtable) {
             }

             @Override
             public void setColorModel(ColorModel colorModel) {
             }

             @Override
             public void setHints(int i) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, byte[] bytes,
int i4,
                                   int i5) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, int[] ints,
int i4,
                                   int i5) {
             }

             @Override
             public void imageComplete(int i) {
             }
         };
         final ImageProducer ip = new ImageProducer() {
             @Override
             public void addConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public boolean isConsumer(ImageConsumer imageConsumer) {
                 return false;
             }

             @Override
             public void removeConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public void startProduction(ImageConsumer imageConsumer) {
             }

             @Override
             public void requestTopDownLeftRightResend(
                     ImageConsumer imageConsumer) {
             }
         };

         ImageFilter filter = new ImageFilter();
         FilteredImageSource fis = new FilteredImageSource(ip, filter);
         Thread t1 = new Thread(() -> {
             try {
                 while (true) {
                     fis.addConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t2 = new Thread(() -> {
             try {
                 while (true) {
                     fis.removeConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t3 = new Thread(() -> {
             try {
                 while (true) {
                     fis.startProduction(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         t1.setDaemon(true);
         t2.setDaemon(true);
         t3.setDaemon(true);

         t1.start();
         t2.start();
         t3.start();

         t1.join(10000);
         if (fail != null) {
             throw new RuntimeException("Test fail", fail);
         }
     }
}

On 25/10/2017 23:58, Prahalad Kumar Narayanan wrote:

> Hello Sergey
>
> Thank you for the suggestion.
>
> I 've updated JBS with below information.
>     1 . Stack trace information as provided by submitter-
>          java.lang.NullPointerException
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
>              at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
>              at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
>      2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
>              . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, October 26, 2017 12:05 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> Hi, Prahalad.
> Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.
>
> On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
>> Hello Everyone
>>
>> Good day to you.
>>
>> Kindly review a fix for the bug
>>       Bug ID: JDK-8188083
>>       Description: Null Pointer Exception in
>> java.awt.image.FilteredImageSource.startProduction
>>
>> Root Cause
>>       . FilteredImageSource implements ImageProducer interface
>>       . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>>       . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>>
>> Exception occurrence
>>       . The submitter has mentioned that there is no test-case to reproduce to this issue.
>>       . Luckily I was able to observe this issue with a "crude" test code
>>             . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>>             . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>>             . Hence the test code isn't added to the webrev.
>>
>> Details on the Fix:
>>       . The concerned methods have been "synchronized" in the fix.
>>       . No new regression failures were observed with this change.
>>
>> Kindly review the change and provide your feedback.
>> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>>
>> Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>>
>> Thank you for your time in review
>> Have a good day
>>
>> Prahalad N.
>>
>>
>>
>>
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Sergey

Thank you for your time in review & suggestion on test-code.

My test code was similar to yours except that it used an ExecutorService & queued Runnable objects on them.
The runnable would execute
    . Either add and remove of ImageConsumer  on FilteredImageSource
    . Or Invoke startProduction on FilteredImageSource

As per API documentation of FilteredImageSource, the methods like- addConsumer, removeConsumer, startProduction shouldn't be invoked from user code directly. Hence, the test case isn't really valid and will not reflect the scenario that led to the exception for the submitter.

I 'm inspecting the other classes that implement ImageProducer interface now- MemoryImageSource, RenderableImageProducer. Both these classes contain 'synchronized' implementations for startProduction method. But we need to be sure that such accesses are not harmful to cause deadlocks as Phil rightly pointed out. I shall respond with my findings soon.

Thank you once again for your time
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, October 27, 2017 10:21 AM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

One more note:
I guess it is possible to create a stable test like below which can reproduce the bug:

import java.awt.image.ColorModel;
import java.awt.image.FilteredImageSource;
import java.awt.image.ImageConsumer;
import java.awt.image.ImageFilter;
import java.awt.image.ImageProducer;
import java.util.Hashtable;

public final class Test {

     private volatile static Throwable fail;

     public static void main(final String[] args) throws InterruptedException {
         final ImageConsumer ic = new ImageConsumer() {
             @Override
             public void setDimensions(int i, int i1) {
             }

             @Override
             public void setProperties(Hashtable<?, ?> hashtable) {
             }

             @Override
             public void setColorModel(ColorModel colorModel) {
             }

             @Override
             public void setHints(int i) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, byte[] bytes, int i4,
                                   int i5) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, int[] ints, int i4,
                                   int i5) {
             }

             @Override
             public void imageComplete(int i) {
             }
         };
         final ImageProducer ip = new ImageProducer() {
             @Override
             public void addConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public boolean isConsumer(ImageConsumer imageConsumer) {
                 return false;
             }

             @Override
             public void removeConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public void startProduction(ImageConsumer imageConsumer) {
             }

             @Override
             public void requestTopDownLeftRightResend(
                     ImageConsumer imageConsumer) {
             }
         };

         ImageFilter filter = new ImageFilter();
         FilteredImageSource fis = new FilteredImageSource(ip, filter);
         Thread t1 = new Thread(() -> {
             try {
                 while (true) {
                     fis.addConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t2 = new Thread(() -> {
             try {
                 while (true) {
                     fis.removeConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t3 = new Thread(() -> {
             try {
                 while (true) {
                     fis.startProduction(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         t1.setDaemon(true);
         t2.setDaemon(true);
         t3.setDaemon(true);

         t1.start();
         t2.start();
         t3.start();

         t1.join(10000);
         if (fail != null) {
             throw new RuntimeException("Test fail", fail);
         }
     }
}

On 25/10/2017 23:58, Prahalad Kumar Narayanan wrote:

> Hello Sergey
>
> Thank you for the suggestion.
>
> I 've updated JBS with below information.
>     1 . Stack trace information as provided by submitter-
>          java.lang.NullPointerException
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
>              at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
>              at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
>      2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
>              . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, October 26, 2017 12:05 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
> java.awt.image.FilteredImageSource.startProduction
>
> Hi, Prahalad.
> Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.
>
> On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
>> Hello Everyone
>>
>> Good day to you.
>>
>> Kindly review a fix for the bug
>>       Bug ID: JDK-8188083
>>       Description: Null Pointer Exception in
>> java.awt.image.FilteredImageSource.startProduction
>>
>> Root Cause
>>       . FilteredImageSource implements ImageProducer interface
>>       . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>>       . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>>
>> Exception occurrence
>>       . The submitter has mentioned that there is no test-case to reproduce to this issue.
>>       . Luckily I was able to observe this issue with a "crude" test code
>>             . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>>             . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>>             . Hence the test code isn't added to the webrev.
>>
>> Details on the Fix:
>>       . The concerned methods have been "synchronized" in the fix.
>>       . No new regression failures were observed with this change.
>>
>> Kindly review the change and provide your feedback.
>> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>>
>> Review Link:
>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>>
>> Thank you for your time in review
>> Have a good day
>>
>> Prahalad N.
>>
>>
>>
>>
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
In reply to this post by Phil Race
Hello Everyone

First, Thanks to Phil & Sergey for their time in review & suggestions.

This is a follow-up on Phil's suggestion to investigate whether the fix proposed in webrev.00 could lead to deadlocks.
Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/

I inspected the code, and wrote code snippets to check the behavior.
Inferences are as follows-
    1. FilteredImageSource helps in producing the image, applying an image filter on the pixels, and sharing the result to the ImageConsumer.
    2. The object holds- An ImageProducer, An ImageFilter, and instantiates a HashTable<ImageConsumer, ImageFilter> for its operation.

    3. The methods of FilteredImageSource are not supposed to be invoked directly from user-code. So how are these methods invoked?
          . startProduction:
                . This method is invoked when a user creates an Image like- Component.createImage(new FilteredImageSource(imgProducer, imgFilter));
                . Such creation could occur on any thread in user code and this internally invokes FilteredImageSource.startProduction through ToolkitImage.
                . The method updates the hash table as required and invokes ImageProducer.startProduction to produce the image.
          . removeConsumer:
                . This method is invoked once ImageProducer is done with image preparation.
                . The exact thread in which the removeConsumer gets invoked depends on how ImageProducer prepares the image.

    4. Is there a possibility of a deadlock with the proposed fix ?
          . In my inspection & observation with code snippets, I found that removeConsumer could be invoked in two ways- Multi-threaded & Single threaded.
          . Multi-threaded-
                . This was observed with FileImageSource as ImageProducer
                . startProduction on this image producer requests image decode operation on a different thread- ImageFetcher thread.
                . Once decoding is complete, removeConsumer is invoked on the ImageFetcher thread.
                . Though startProduction & removeConsumer are invoked on different threads, there is no possibility of a deadlock as the code flow from startProduction returns immediately after posting the request to decode image onto ImageFetcher 's queue.
          . Single threaded-
                . This was observed with OffscreenImageSource as ImageProducer
                . startProduction and removeConsumer are invoked on the same thread.
                . Since re-entrant threads acquire the locks that they already hold, this wouldn't result in a deadlock.
          . Thus, in both cases I don't see possibility of a deadlock. Hence, we can consider the change in webrev.00 safe to fix the issue.

    5. I have run JTreg test cases before and after the proposed change on java/awt/image/ and javax/swing (where ImageFilter is used).
         No new regressions were observed during the test.

Kindly review by assessing the above information.
If you believe, the fix could be approved, kindly review the CSR as well.

Thank you once again for your time
Have a good day

Prahalad

----- Original Message -----
From: Philip Race
Sent: Thursday, October 26, 2017 9:30 PM
To: Prahalad Kumar Narayanan
Cc: Sergey Bylokhov; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

The screenshot shows you directly calling this method in your test which
the documentation says you are not supposed to do.
So I am not able to be 100% sure that the test you have re-creates what
the submitter saw .. in his stack trace you have below it seems to be valid.

But to have a NPE at line 181 is odd.
 181             proxies.put(ic, imgf);

What is null ? proxies was just initialised in this same method so
I don't see how it can be null unless something elsewhere is
resetting it to null.

The only place I see that is removeConsumer
 138     public synchronized void removeConsumer(ImageConsumer ic) {
 139         if (proxies != null) {
 140             ImageFilter imgf =  proxies.get(ic);
 141             if (imgf != null) {
 142                 src.removeConsumer(imgf);
 143                 proxies.remove(ic);
 144                 if (proxies.isEmpty()) {
 145                     proxies = null;
 146                 }
 147             }
 148         }
 149     }

Now that method is synchronized .. OK so I think it might make sense
to mark the additional methods synchronized too but then I automatically
worry about introducing a deadlock.

I can't say for sure that you have done so however and likely there
are no nested locks here so it is probably OK but please run as many
tests as you can find to try to ensure that.

Adding or removing sychronized is binary compatible but since
these are public API methods you should still do a CSR
before pushing if this ends up being the approved fix.

-phil.



On 10/25/17, 11:58 PM, Prahalad Kumar Narayanan wrote:
Hello Sergey

Thank you for the suggestion.

I 've updated JBS with below information.
   1 . Stack trace information as provided by submitter-
        java.lang.NullPointerException
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
            at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
            at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
            at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
    2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
            . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.

Thank you
Have a good day

Prahalad N.

-----Original Message-----
From: Sergey Bylokhov
Sent: Thursday, October 26, 2017 12:05 PM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Hi, Prahalad.
Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.

On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
Hello Everyone

Good day to you.

Kindly review a fix for the bug
     Bug ID: JDK-8188083
     Description: Null Pointer Exception in
java.awt.image.FilteredImageSource.startProduction

Root Cause
     . FilteredImageSource implements ImageProducer interface
     . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
     . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.

Exception occurrence
     . The submitter has mentioned that there is no test-case to reproduce to this issue.
     . Luckily I was able to observe this issue with a "crude" test code
           . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
           . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
           . Hence the test code isn't added to the webrev.

Details on the Fix:
     . The concerned methods have been "synchronized" in the fix.
     . No new regression failures were observed with this change.

Kindly review the change and provide your feedback.
In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.

Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/

Thank you for your time in review
Have a good day

Prahalad N.







--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Phil Race
Ok to the fix. But Sergey need to respond as to whether the answer about
the test satisfies him.

-phil.

On 11/09/2017 02:18 AM, Prahalad Kumar Narayanan wrote:

> Hello Everyone
>
> First, Thanks to Phil & Sergey for their time in review & suggestions.
>
> This is a follow-up on Phil's suggestion to investigate whether the fix proposed in webrev.00 could lead to deadlocks.
> Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> I inspected the code, and wrote code snippets to check the behavior.
> Inferences are as follows-
>      1. FilteredImageSource helps in producing the image, applying an image filter on the pixels, and sharing the result to the ImageConsumer.
>      2. The object holds- An ImageProducer, An ImageFilter, and instantiates a HashTable<ImageConsumer, ImageFilter> for its operation.
>
>      3. The methods of FilteredImageSource are not supposed to be invoked directly from user-code. So how are these methods invoked?
>            . startProduction:
>                  . This method is invoked when a user creates an Image like- Component.createImage(new FilteredImageSource(imgProducer, imgFilter));
>                  . Such creation could occur on any thread in user code and this internally invokes FilteredImageSource.startProduction through ToolkitImage.
>                  . The method updates the hash table as required and invokes ImageProducer.startProduction to produce the image.
>            . removeConsumer:
>                  . This method is invoked once ImageProducer is done with image preparation.
>                  . The exact thread in which the removeConsumer gets invoked depends on how ImageProducer prepares the image.
>
>      4. Is there a possibility of a deadlock with the proposed fix ?
>            . In my inspection & observation with code snippets, I found that removeConsumer could be invoked in two ways- Multi-threaded & Single threaded.
>            . Multi-threaded-
>                  . This was observed with FileImageSource as ImageProducer
>                  . startProduction on this image producer requests image decode operation on a different thread- ImageFetcher thread.
>                  . Once decoding is complete, removeConsumer is invoked on the ImageFetcher thread.
>                  . Though startProduction & removeConsumer are invoked on different threads, there is no possibility of a deadlock as the code flow from startProduction returns immediately after posting the request to decode image onto ImageFetcher 's queue.
>            . Single threaded-
>                  . This was observed with OffscreenImageSource as ImageProducer
>                  . startProduction and removeConsumer are invoked on the same thread.
>                  . Since re-entrant threads acquire the locks that they already hold, this wouldn't result in a deadlock.
>            . Thus, in both cases I don't see possibility of a deadlock. Hence, we can consider the change in webrev.00 safe to fix the issue.
>
>      5. I have run JTreg test cases before and after the proposed change on java/awt/image/ and javax/swing (where ImageFilter is used).
>           No new regressions were observed during the test.
>
> Kindly review by assessing the above information.
> If you believe, the fix could be approved, kindly review the CSR as well.
>
> Thank you once again for your time
> Have a good day
>
> Prahalad
>
> ----- Original Message -----
> From: Philip Race
> Sent: Thursday, October 26, 2017 9:30 PM
> To: Prahalad Kumar Narayanan
> Cc: Sergey Bylokhov; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> The screenshot shows you directly calling this method in your test which
> the documentation says you are not supposed to do.
> So I am not able to be 100% sure that the test you have re-creates what
> the submitter saw .. in his stack trace you have below it seems to be valid.
>
> But to have a NPE at line 181 is odd.
>   181             proxies.put(ic, imgf);
>
> What is null ? proxies was just initialised in this same method so
> I don't see how it can be null unless something elsewhere is
> resetting it to null.
>
> The only place I see that is removeConsumer
>   138     public synchronized void removeConsumer(ImageConsumer ic) {
>   139         if (proxies != null) {
>   140             ImageFilter imgf =  proxies.get(ic);
>   141             if (imgf != null) {
>   142                 src.removeConsumer(imgf);
>   143                 proxies.remove(ic);
>   144                 if (proxies.isEmpty()) {
>   145                     proxies = null;
>   146                 }
>   147             }
>   148         }
>   149     }
>
> Now that method is synchronized .. OK so I think it might make sense
> to mark the additional methods synchronized too but then I automatically
> worry about introducing a deadlock.
>
> I can't say for sure that you have done so however and likely there
> are no nested locks here so it is probably OK but please run as many
> tests as you can find to try to ensure that.
>
> Adding or removing sychronized is binary compatible but since
> these are public API methods you should still do a CSR
> before pushing if this ends up being the approved fix.
>
> -phil.
>
>
>
> On 10/25/17, 11:58 PM, Prahalad Kumar Narayanan wrote:
> Hello Sergey
>
> Thank you for the suggestion.
>
> I 've updated JBS with below information.
>     1 . Stack trace information as provided by submitter-
>          java.lang.NullPointerException
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
>              at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
>              at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
>      2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
>              . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, October 26, 2017 12:05 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> Hi, Prahalad.
> Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.
>
> On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
> Hello Everyone
>
> Good day to you.
>
> Kindly review a fix for the bug
>       Bug ID: JDK-8188083
>       Description: Null Pointer Exception in
> java.awt.image.FilteredImageSource.startProduction
>
> Root Cause
>       . FilteredImageSource implements ImageProducer interface
>       . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>       . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>
> Exception occurrence
>       . The submitter has mentioned that there is no test-case to reproduce to this issue.
>       . Luckily I was able to observe this issue with a "crude" test code
>             . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>             . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>             . Hence the test code isn't added to the webrev.
>
> Details on the Fix:
>       . The concerned methods have been "synchronized" in the fix.
>       . No new regression failures were observed with this change.
>
> Kindly review the change and provide your feedback.
> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>
> Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> Thank you for your time in review
> Have a good day
>
> Prahalad N.
>
>
>
>
>
>
>
> --
> Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Everyone

First, Thanks to Phil for his time in review.

Based on discussions with Sergey, I 've now updated the fix with a test case.
The changes are available for review under:
http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/

Brief on changes:
. A test case has been provided. No other changes to source code since last revision of the fix.
. The test case creates 30 ImageIcon(s) in different threads and checks if the creation of ImageIcon(s) results in Null pointer exception in concerned method.
. The test uses existing image file in the repository. Hence the webrev doesn’t contain additional image file.

Kindly review the changes and provide your feedback.

Thank you once again for your time in review
Have a good day

Prahalad

-----Original Message-----
From: Phil Race
Sent: Friday, November 10, 2017 2:30 AM
To: Prahalad Kumar Narayanan
Cc: Sergey Bylokhov; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Ok to the fix. But Sergey need to respond as to whether the answer about the test satisfies him.

-phil.

On 11/09/2017 02:18 AM, Prahalad Kumar Narayanan wrote:

> Hello Everyone
>
> First, Thanks to Phil & Sergey for their time in review & suggestions.
>
> This is a follow-up on Phil's suggestion to investigate whether the fix proposed in webrev.00 could lead to deadlocks.
> Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> I inspected the code, and wrote code snippets to check the behavior.
> Inferences are as follows-
>      1. FilteredImageSource helps in producing the image, applying an image filter on the pixels, and sharing the result to the ImageConsumer.
>      2. The object holds- An ImageProducer, An ImageFilter, and instantiates a HashTable<ImageConsumer, ImageFilter> for its operation.
>
>      3. The methods of FilteredImageSource are not supposed to be invoked directly from user-code. So how are these methods invoked?
>            . startProduction:
>                  . This method is invoked when a user creates an Image like- Component.createImage(new FilteredImageSource(imgProducer, imgFilter));
>                  . Such creation could occur on any thread in user code and this internally invokes FilteredImageSource.startProduction through ToolkitImage.
>                  . The method updates the hash table as required and invokes ImageProducer.startProduction to produce the image.
>            . removeConsumer:
>                  . This method is invoked once ImageProducer is done with image preparation.
>                  . The exact thread in which the removeConsumer gets invoked depends on how ImageProducer prepares the image.
>
>      4. Is there a possibility of a deadlock with the proposed fix ?
>            . In my inspection & observation with code snippets, I found that removeConsumer could be invoked in two ways- Multi-threaded & Single threaded.
>            . Multi-threaded-
>                  . This was observed with FileImageSource as ImageProducer
>                  . startProduction on this image producer requests image decode operation on a different thread- ImageFetcher thread.
>                  . Once decoding is complete, removeConsumer is invoked on the ImageFetcher thread.
>                  . Though startProduction & removeConsumer are invoked on different threads, there is no possibility of a deadlock as the code flow from startProduction returns immediately after posting the request to decode image onto ImageFetcher 's queue.
>            . Single threaded-
>                  . This was observed with OffscreenImageSource as ImageProducer
>                  . startProduction and removeConsumer are invoked on the same thread.
>                  . Since re-entrant threads acquire the locks that they already hold, this wouldn't result in a deadlock.
>            . Thus, in both cases I don't see possibility of a deadlock. Hence, we can consider the change in webrev.00 safe to fix the issue.
>
>      5. I have run JTreg test cases before and after the proposed change on java/awt/image/ and javax/swing (where ImageFilter is used).
>           No new regressions were observed during the test.
>
> Kindly review by assessing the above information.
> If you believe, the fix could be approved, kindly review the CSR as well.
>
> Thank you once again for your time
> Have a good day
>
> Prahalad
>
> ----- Original Message -----
> From: Philip Race
> Sent: Thursday, October 26, 2017 9:30 PM
> To: Prahalad Kumar Narayanan
> Cc: Sergey Bylokhov; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
> java.awt.image.FilteredImageSource.startProduction
>
> The screenshot shows you directly calling this method in your test
> which the documentation says you are not supposed to do.
> So I am not able to be 100% sure that the test you have re-creates
> what the submitter saw .. in his stack trace you have below it seems to be valid.
>
> But to have a NPE at line 181 is odd.
>   181             proxies.put(ic, imgf);
>
> What is null ? proxies was just initialised in this same method so I
> don't see how it can be null unless something elsewhere is resetting
> it to null.
>
> The only place I see that is removeConsumer
>   138     public synchronized void removeConsumer(ImageConsumer ic) {
>   139         if (proxies != null) {
>   140             ImageFilter imgf =  proxies.get(ic);
>   141             if (imgf != null) {
>   142                 src.removeConsumer(imgf);
>   143                 proxies.remove(ic);
>   144                 if (proxies.isEmpty()) {
>   145                     proxies = null;
>   146                 }
>   147             }
>   148         }
>   149     }
>
> Now that method is synchronized .. OK so I think it might make sense
> to mark the additional methods synchronized too but then I
> automatically worry about introducing a deadlock.
>
> I can't say for sure that you have done so however and likely there
> are no nested locks here so it is probably OK but please run as many
> tests as you can find to try to ensure that.
>
> Adding or removing sychronized is binary compatible but since these
> are public API methods you should still do a CSR before pushing if
> this ends up being the approved fix.
>
> -phil.
>
>
>
> On 10/25/17, 11:58 PM, Prahalad Kumar Narayanan wrote:
> Hello Sergey
>
> Thank you for the suggestion.
>
> I 've updated JBS with below information.
>     1 . Stack trace information as provided by submitter-
>          java.lang.NullPointerException
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:181)
>              at java.awt.image.FilteredImageSource.startProduction(FilteredImageSource.java:183)
>              at sun.awt.image.ImageRepresentation.startProduction(ImageRepresentation.java:732)
>              at sun.awt.image.ToolkitImage.addWatcher(ToolkitImage.java:221)
>      2. Screenshot showing the occurrence of the exception at FilteredImageSource.java: 181.
>              . The image also shows the test passing with JDK10 containing the fix on VM running Linux Ubuntu x64.
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Thursday, October 26, 2017 12:05 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
> java.awt.image.FilteredImageSource.startProduction
>
> Hi, Prahalad.
> Can you please add a stack trace which include a line numbers to the bug report? Currently it is unclear in what line an exception is occurred.
>
> On 25/10/2017 21:07, Prahalad Kumar Narayanan wrote:
> Hello Everyone
>
> Good day to you.
>
> Kindly review a fix for the bug
>       Bug ID: JDK-8188083
>       Description: Null Pointer Exception in
> java.awt.image.FilteredImageSource.startProduction
>
> Root Cause
>       . FilteredImageSource implements ImageProducer interface
>       . All the methods of FilteredImageSource operate on a common data -HashTable, but only a few are synchronized methods.
>       . Thus, when synchronized & un-synchronized methods access / modify the hash table in a multi-threaded scenario, it renders the class vulnerable to this exception.
>
> Exception occurrence
>       . The submitter has mentioned that there is no test-case to reproduce to this issue.
>       . Luckily I was able to observe this issue with a "crude" test code
>             . The test triggered 7k threads in an ExecutorService randomly adding/ removing/ invoking startProduction on FilteredImageSource object.
>             . I didn't feel it robust enough to be added as a part of regression tests. I tried optimizing the test but in vain.
>             . Hence the test code isn't added to the webrev.
>
> Details on the Fix:
>       . The concerned methods have been "synchronized" in the fix.
>       . No new regression failures were observed with this change.
>
> Kindly review the change and provide your feedback.
> In addition, kindly suggest whether this requires CSR as it adds "synchronized" to method signature.
>
> Review Link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.00/
>
> Thank you for your time in review
> Have a good day
>
> Prahalad N.
>
>
>
>
>
>
>
> --
> Best regards, Sergey.

Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
Hi,Prahalad.
On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
> Based on discussions with Sergey, I 've now updated the fix with a test case.
> The changes are available for review under:
> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/

This version of the test is always passed before the fix(I have checked
w/ and w/o jtreg).

--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Sergey

Thank you for your time in review.
Yes..! As you rightly pointed, the test passes before the fix as well.

Reason is that, the occurrence of this bug is rare.
    . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
    . Though the occurrence is rare, submitter is expecting a fix for this issue.

So what does this test-case resolve ?
    . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
    . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
    . The test case helps to address both but in a passive way. Why do I say passive ?
          . First, the test provides a hope to detect & report NPE though its occurrence is rare.
          . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
 
Thank you once again for your time
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Monday, November 27, 2017 9:11 PM
To: Prahalad Kumar Narayanan; 2d-dev
Cc: Philip Race
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Hi,Prahalad.
On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
> Based on discussions with Sergey, I 've now updated the fix with a test case.
> The changes are available for review under:
> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/

This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).

--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
On 27/11/2017 22:12, Prahalad Kumar Narayanan wrote:
> Yes..! As you rightly pointed, the test passes before the fix as well.

Then I suggest to additionally reuse a test which I sent previously, it
is fail almost always before the fix. The test which fails one time in
10000 runs will be never fixed, or such bugs will be closed as not
reproducible.

>
> Reason is that, the occurrence of this bug is rare.
>      . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
>      . Though the occurrence is rare, submitter is expecting a fix for this issue.
>
> So what does this test-case resolve ?
>      . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
>      . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
>      . The test case helps to address both but in a passive way. Why do I say passive ?
>            . First, the test provides a hope to detect & report NPE though its occurrence is rare.
>            . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
>  
> Thank you once again for your time
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Monday, November 27, 2017 9:11 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Cc: Philip Race
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> Hi,Prahalad.
> On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
>> Based on discussions with Sergey, I 've now updated the fix with a test case.
>> The changes are available for review under:
>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/
>
> This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Sergey

Thank you for your review response.

I had one version of the test that resembled your test code.
I uploaded the result (screenshot) of that test case with/ without fix on JBS.

The only point of concern - API documentation mentions that the methods of FilteredImageSource are not supposed to be invoked directly from user code. If the methods are invoked directly from user code, the result is un-defined. Since our JTreg test cases cannot violate the documented guide, I refrained from attaching the test case in the first webrev.

Kindly let me know your views to this.

Thank you once again
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 28, 2017 12:46 PM
To: Prahalad Kumar Narayanan; 2d-dev
Cc: Philip Race
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

On 27/11/2017 22:12, Prahalad Kumar Narayanan wrote:
> Yes..! As you rightly pointed, the test passes before the fix as well.

Then I suggest to additionally reuse a test which I sent previously, it is fail almost always before the fix. The test which fails one time in
10000 runs will be never fixed, or such bugs will be closed as not reproducible.

>
> Reason is that, the occurrence of this bug is rare.
>      . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
>      . Though the occurrence is rare, submitter is expecting a fix for this issue.
>
> So what does this test-case resolve ?
>      . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
>      . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
>      . The test case helps to address both but in a passive way. Why do I say passive ?
>            . First, the test provides a hope to detect & report NPE though its occurrence is rare.
>            . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
>  
> Thank you once again for your time
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Monday, November 27, 2017 9:11 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Cc: Philip Race
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> Hi,Prahalad.
> On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
>> Based on discussions with Sergey, I 've now updated the fix with a test case.
>> The changes are available for review under:
>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/
>
> This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
The test can use any part of our API. It is a small part of the code
which can confirm/verify the fix. It should not be considered as a
user's code and it could emulate behavior of classes inside jdk.
If you do not like to call these methods directly you can do this like this:

import java.awt.Graphics;
import java.awt.Image;
import java.awt.image.ColorModel;
import java.awt.image.FilteredImageSource;
import java.awt.image.ImageConsumer;
import java.awt.image.ImageFilter;
import java.awt.image.ImageObserver;
import java.awt.image.ImageProducer;
import java.util.Hashtable;

public final class Test {

     private volatile static Throwable fail;

     public static void main(final String[] args) throws
                                                  InterruptedException {

         final ImageConsumer ic = new ImageConsumer() {
             @Override
             public void setDimensions(int i, int i1) {
             }

             @Override
             public void setProperties(Hashtable<?, ?> hashtable) {
             }

             @Override
             public void setColorModel(ColorModel colorModel) {
             }

             @Override
             public void setHints(int i) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, byte[] bytes,
                                   int i4,
                                   int i5) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, int[] ints,
                                   int i4,
                                   int i5) {
             }

             @Override
             public void imageComplete(int i) {
             }
         };
         final ImageProducer ip = new ImageProducer() {
             @Override
             public void addConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public boolean isConsumer(ImageConsumer imageConsumer) {
                 return false;
             }

             @Override
             public void removeConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public void startProduction(ImageConsumer imageConsumer) {
             }

             @Override
             public void requestTopDownLeftRightResend(
                     ImageConsumer imageConsumer) {
             }
         };

         final Image image = new Image() {
             ImageFilter filter = new ImageFilter();
             ImageProducer producer =  new FilteredImageSource(ip, filter);

             @Override
             public int getWidth(ImageObserver observer) {
                 return 100;
             }

             @Override
             public int getHeight(ImageObserver observer) {
                 return 100;
             }

             @Override
             public ImageProducer getSource() {
                 return producer;
             }

             @Override
             public Graphics getGraphics() {
                 throw new UnsupportedOperationException();
             }

             @Override
             public Object getProperty(String name, ImageObserver
observer) {
                 return null;
             }
         };

         Thread t1 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().addConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t2 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().removeConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t3 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().startProduction(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         t1.setDaemon(true);
         t2.setDaemon(true);
         t3.setDaemon(true);

         t1.start();
         t2.start();
         t3.start();

         t1.join(10000);
         if (fail != null) {
             throw new RuntimeException("Test fail", fail);
         }
     }
}


On 27/11/2017 23:31, Prahalad Kumar Narayanan wrote:

> Hello Sergey
>
> Thank you for your review response.
>
> I had one version of the test that resembled your test code.
> I uploaded the result (screenshot) of that test case with/ without fix on JBS.
>
> The only point of concern - API documentation mentions that the methods of FilteredImageSource are not supposed to be invoked directly from user code. If the methods are invoked directly from user code, the result is un-defined. Since our JTreg test cases cannot violate the documented guide, I refrained from attaching the test case in the first webrev.
>
> Kindly let me know your views to this.
>
> Thank you once again
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, November 28, 2017 12:46 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Cc: Philip Race
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> On 27/11/2017 22:12, Prahalad Kumar Narayanan wrote:
>> Yes..! As you rightly pointed, the test passes before the fix as well.
>
> Then I suggest to additionally reuse a test which I sent previously, it is fail almost always before the fix. The test which fails one time in
> 10000 runs will be never fixed, or such bugs will be closed as not reproducible.
>
>>
>> Reason is that, the occurrence of this bug is rare.
>>       . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
>>       . Though the occurrence is rare, submitter is expecting a fix for this issue.
>>
>> So what does this test-case resolve ?
>>       . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
>>       . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
>>       . The test case helps to address both but in a passive way. Why do I say passive ?
>>             . First, the test provides a hope to detect & report NPE though its occurrence is rare.
>>             . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
>>    
>> Thank you once again for your time
>> Have a good day
>>
>> Prahalad
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Monday, November 27, 2017 9:11 PM
>> To: Prahalad Kumar Narayanan; 2d-dev
>> Cc: Philip Race
>> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>>
>> Hi,Prahalad.
>> On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
>>> Based on discussions with Sergey, I 've now updated the fix with a test case.
>>> The changes are available for review under:
>>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/
>>
>> This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).
>>
>> --
>> Best regards, Sergey.
>>
>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Prahalad kumar Narayanan
Hello Everyone

First, Thanks to Sergey for his detailed suggestion on test-case approach.

I've modified the test-case keeping Sergey's suggestion as a reference.
Few subtle changes are-
    . Created explicit classes rather than using anonymous class definitions
    . Modified minimum test duration from 10 seconds to 5 seconds.
The test-case now reproduces this issue without the proposed fix. (Tested on win7 and Ubuntu VM)

Besides this, I 've updated Javadoc comments for FilteredImageSource as requested by Joe Darcy in CSR review.
    . Since every method is synchronized, I decided to update javadoc comments for the class.
    . The approach is similar to how java.util.HashTable captures the synchronized contract of its methods.

The javadoc modification is as follows-
@@ -35,7 +35,8 @@
 /**
  * This class is an implementation of the ImageProducer interface which
  * takes an existing image and a filter object and uses them to produce
- * image data for a new filtered version of the original image.
+ * image data for a new filtered version of the original image. Furthermore,
+ * {@code FilteredImageSource} is synchronized for thread-safety.
  * Here is an example which filters an image by swapping the red and

Kindly review the above changes at your convenience and share your views
    Webrev link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.02/

Thank you for your time
Have a good day

Prahalad

-----Original Message-----
From: Sergey Bylokhov
Sent: Tuesday, November 28, 2017 11:14 PM
To: Prahalad Kumar Narayanan; 2d-dev
Cc: Philip Race
Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

The test can use any part of our API. It is a small part of the code which can confirm/verify the fix. It should not be considered as a user's code and it could emulate behavior of classes inside jdk.
If you do not like to call these methods directly you can do this like this:

import java.awt.Graphics;
import java.awt.Image;
import java.awt.image.ColorModel;
import java.awt.image.FilteredImageSource;
import java.awt.image.ImageConsumer;
import java.awt.image.ImageFilter;
import java.awt.image.ImageObserver;
import java.awt.image.ImageProducer;
import java.util.Hashtable;

public final class Test {

     private volatile static Throwable fail;

     public static void main(final String[] args) throws
                                                  InterruptedException {

         final ImageConsumer ic = new ImageConsumer() {
             @Override
             public void setDimensions(int i, int i1) {
             }

             @Override
             public void setProperties(Hashtable<?, ?> hashtable) {
             }

             @Override
             public void setColorModel(ColorModel colorModel) {
             }

             @Override
             public void setHints(int i) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, byte[] bytes,
                                   int i4,
                                   int i5) {
             }

             @Override
             public void setPixels(int i, int i1, int i2, int i3,
                                   ColorModel colorModel, int[] ints,
                                   int i4,
                                   int i5) {
             }

             @Override
             public void imageComplete(int i) {
             }
         };
         final ImageProducer ip = new ImageProducer() {
             @Override
             public void addConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public boolean isConsumer(ImageConsumer imageConsumer) {
                 return false;
             }

             @Override
             public void removeConsumer(ImageConsumer imageConsumer) {
             }

             @Override
             public void startProduction(ImageConsumer imageConsumer) {
             }

             @Override
             public void requestTopDownLeftRightResend(
                     ImageConsumer imageConsumer) {
             }
         };

         final Image image = new Image() {
             ImageFilter filter = new ImageFilter();
             ImageProducer producer =  new FilteredImageSource(ip, filter);

             @Override
             public int getWidth(ImageObserver observer) {
                 return 100;
             }

             @Override
             public int getHeight(ImageObserver observer) {
                 return 100;
             }

             @Override
             public ImageProducer getSource() {
                 return producer;
             }

             @Override
             public Graphics getGraphics() {
                 throw new UnsupportedOperationException();
             }

             @Override
             public Object getProperty(String name, ImageObserver
observer) {
                 return null;
             }
         };

         Thread t1 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().addConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t2 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().removeConsumer(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         Thread t3 = new Thread(() -> {
             try {
                 while (true) {
                     image.getSource().startProduction(ic);
                 }
             } catch (Throwable t) {
                 fail = t;
             }
         });
         t1.setDaemon(true);
         t2.setDaemon(true);
         t3.setDaemon(true);

         t1.start();
         t2.start();
         t3.start();

         t1.join(10000);
         if (fail != null) {
             throw new RuntimeException("Test fail", fail);
         }
     }
}


On 27/11/2017 23:31, Prahalad Kumar Narayanan wrote:

> Hello Sergey
>
> Thank you for your review response.
>
> I had one version of the test that resembled your test code.
> I uploaded the result (screenshot) of that test case with/ without fix on JBS.
>
> The only point of concern - API documentation mentions that the methods of FilteredImageSource are not supposed to be invoked directly from user code. If the methods are invoked directly from user code, the result is un-defined. Since our JTreg test cases cannot violate the documented guide, I refrained from attaching the test case in the first webrev.
>
> Kindly let me know your views to this.
>
> Thank you once again
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, November 28, 2017 12:46 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Cc: Philip Race
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
> java.awt.image.FilteredImageSource.startProduction
>
> On 27/11/2017 22:12, Prahalad Kumar Narayanan wrote:
>> Yes..! As you rightly pointed, the test passes before the fix as well.
>
> Then I suggest to additionally reuse a test which I sent previously,
> it is fail almost always before the fix. The test which fails one time
> in
> 10000 runs will be never fixed, or such bugs will be closed as not reproducible.
>
>>
>> Reason is that, the occurrence of this bug is rare.
>>       . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
>>       . Though the occurrence is rare, submitter is expecting a fix for this issue.
>>
>> So what does this test-case resolve ?
>>       . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
>>       . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
>>       . The test case helps to address both but in a passive way. Why do I say passive ?
>>             . First, the test provides a hope to detect & report NPE though its occurrence is rare.
>>             . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
>>    
>> Thank you once again for your time
>> Have a good day
>>
>> Prahalad
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Monday, November 27, 2017 9:11 PM
>> To: Prahalad Kumar Narayanan; 2d-dev
>> Cc: Philip Race
>> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
>> java.awt.image.FilteredImageSource.startProduction
>>
>> Hi,Prahalad.
>> On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
>>> Based on discussions with Sergey, I 've now updated the fix with a test case.
>>> The changes are available for review under:
>>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/
>>
>> This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).
>>
>> --
>> Best regards, Sergey.
>>
>
>


--
Best regards, Sergey.
Reply | Threaded
Open this post in threaded view
|

Re: [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction

Sergey Bylokhov
Looks fine, thank you.

On 29/11/2017 04:44, Prahalad Kumar Narayanan wrote:

> Hello Everyone
>
> First, Thanks to Sergey for his detailed suggestion on test-case approach.
>
> I've modified the test-case keeping Sergey's suggestion as a reference.
> Few subtle changes are-
>      . Created explicit classes rather than using anonymous class definitions
>      . Modified minimum test duration from 10 seconds to 5 seconds.
> The test-case now reproduces this issue without the proposed fix. (Tested on win7 and Ubuntu VM)
>
> Besides this, I 've updated Javadoc comments for FilteredImageSource as requested by Joe Darcy in CSR review.
>      . Since every method is synchronized, I decided to update javadoc comments for the class.
>      . The approach is similar to how java.util.HashTable captures the synchronized contract of its methods.
>
> The javadoc modification is as follows-
> @@ -35,7 +35,8 @@
>   /**
>    * This class is an implementation of the ImageProducer interface which
>    * takes an existing image and a filter object and uses them to produce
> - * image data for a new filtered version of the original image.
> + * image data for a new filtered version of the original image. Furthermore,
> + * {@code FilteredImageSource} is synchronized for thread-safety.
>    * Here is an example which filters an image by swapping the red and
>
> Kindly review the above changes at your convenience and share your views
>      Webrev link: http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.02/
>
> Thank you for your time
> Have a good day
>
> Prahalad
>
> -----Original Message-----
> From: Sergey Bylokhov
> Sent: Tuesday, November 28, 2017 11:14 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Cc: Philip Race
> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in java.awt.image.FilteredImageSource.startProduction
>
> The test can use any part of our API. It is a small part of the code which can confirm/verify the fix. It should not be considered as a user's code and it could emulate behavior of classes inside jdk.
> If you do not like to call these methods directly you can do this like this:
>
> import java.awt.Graphics;
> import java.awt.Image;
> import java.awt.image.ColorModel;
> import java.awt.image.FilteredImageSource;
> import java.awt.image.ImageConsumer;
> import java.awt.image.ImageFilter;
> import java.awt.image.ImageObserver;
> import java.awt.image.ImageProducer;
> import java.util.Hashtable;
>
> public final class Test {
>
>       private volatile static Throwable fail;
>
>       public static void main(final String[] args) throws
>                                                    InterruptedException {
>
>           final ImageConsumer ic = new ImageConsumer() {
>               @Override
>               public void setDimensions(int i, int i1) {
>               }
>
>               @Override
>               public void setProperties(Hashtable<?, ?> hashtable) {
>               }
>
>               @Override
>               public void setColorModel(ColorModel colorModel) {
>               }
>
>               @Override
>               public void setHints(int i) {
>               }
>
>               @Override
>               public void setPixels(int i, int i1, int i2, int i3,
>                                     ColorModel colorModel, byte[] bytes,
>                                     int i4,
>                                     int i5) {
>               }
>
>               @Override
>               public void setPixels(int i, int i1, int i2, int i3,
>                                     ColorModel colorModel, int[] ints,
>                                     int i4,
>                                     int i5) {
>               }
>
>               @Override
>               public void imageComplete(int i) {
>               }
>           };
>           final ImageProducer ip = new ImageProducer() {
>               @Override
>               public void addConsumer(ImageConsumer imageConsumer) {
>               }
>
>               @Override
>               public boolean isConsumer(ImageConsumer imageConsumer) {
>                   return false;
>               }
>
>               @Override
>               public void removeConsumer(ImageConsumer imageConsumer) {
>               }
>
>               @Override
>               public void startProduction(ImageConsumer imageConsumer) {
>               }
>
>               @Override
>               public void requestTopDownLeftRightResend(
>                       ImageConsumer imageConsumer) {
>               }
>           };
>
>           final Image image = new Image() {
>               ImageFilter filter = new ImageFilter();
>               ImageProducer producer =  new FilteredImageSource(ip, filter);
>
>               @Override
>               public int getWidth(ImageObserver observer) {
>                   return 100;
>               }
>
>               @Override
>               public int getHeight(ImageObserver observer) {
>                   return 100;
>               }
>
>               @Override
>               public ImageProducer getSource() {
>                   return producer;
>               }
>
>               @Override
>               public Graphics getGraphics() {
>                   throw new UnsupportedOperationException();
>               }
>
>               @Override
>               public Object getProperty(String name, ImageObserver
> observer) {
>                   return null;
>               }
>           };
>
>           Thread t1 = new Thread(() -> {
>               try {
>                   while (true) {
>                       image.getSource().addConsumer(ic);
>                   }
>               } catch (Throwable t) {
>                   fail = t;
>               }
>           });
>           Thread t2 = new Thread(() -> {
>               try {
>                   while (true) {
>                       image.getSource().removeConsumer(ic);
>                   }
>               } catch (Throwable t) {
>                   fail = t;
>               }
>           });
>           Thread t3 = new Thread(() -> {
>               try {
>                   while (true) {
>                       image.getSource().startProduction(ic);
>                   }
>               } catch (Throwable t) {
>                   fail = t;
>               }
>           });
>           t1.setDaemon(true);
>           t2.setDaemon(true);
>           t3.setDaemon(true);
>
>           t1.start();
>           t2.start();
>           t3.start();
>
>           t1.join(10000);
>           if (fail != null) {
>               throw new RuntimeException("Test fail", fail);
>           }
>       }
> }
>
>
> On 27/11/2017 23:31, Prahalad Kumar Narayanan wrote:
>> Hello Sergey
>>
>> Thank you for your review response.
>>
>> I had one version of the test that resembled your test code.
>> I uploaded the result (screenshot) of that test case with/ without fix on JBS.
>>
>> The only point of concern - API documentation mentions that the methods of FilteredImageSource are not supposed to be invoked directly from user code. If the methods are invoked directly from user code, the result is un-defined. Since our JTreg test cases cannot violate the documented guide, I refrained from attaching the test case in the first webrev.
>>
>> Kindly let me know your views to this.
>>
>> Thank you once again
>> Have a good day
>>
>> Prahalad
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: Tuesday, November 28, 2017 12:46 PM
>> To: Prahalad Kumar Narayanan; 2d-dev
>> Cc: Philip Race
>> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
>> java.awt.image.FilteredImageSource.startProduction
>>
>> On 27/11/2017 22:12, Prahalad Kumar Narayanan wrote:
>>> Yes..! As you rightly pointed, the test passes before the fix as well.
>>
>> Then I suggest to additionally reuse a test which I sent previously,
>> it is fail almost always before the fix. The test which fails one time
>> in
>> 10000 runs will be never fixed, or such bugs will be closed as not reproducible.
>>
>>>
>>> Reason is that, the occurrence of this bug is rare.
>>>        . Submitter has observed only 15 occurrences of the exception with over 10,000 instances per month.
>>>        . Though the occurrence is rare, submitter is expecting a fix for this issue.
>>>
>>> So what does this test-case resolve ?
>>>        . Our change adds "synchronized" contract to the method to fix the Null pointer exception.
>>>        . To test the changes, we should check for both- Null Pointer Exception and any deadlock that could arise from "synchronized" contract.
>>>        . The test case helps to address both but in a passive way. Why do I say passive ?
>>>              . First, the test provides a hope to detect & report NPE though its occurrence is rare.
>>>              . Second, any deadlock arising from the concerned method, will cause the test to fail upon time-out.
>>>    
>>> Thank you once again for your time
>>> Have a good day
>>>
>>> Prahalad
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: Monday, November 27, 2017 9:11 PM
>>> To: Prahalad Kumar Narayanan; 2d-dev
>>> Cc: Philip Race
>>> Subject: Re: [OpenJDK 2D-Dev] [10] RFR: JDK-8188083- NPE in
>>> java.awt.image.FilteredImageSource.startProduction
>>>
>>> Hi,Prahalad.
>>> On 24/11/2017 21:40, Prahalad Kumar Narayanan wrote:
>>>> Based on discussions with Sergey, I 've now updated the fix with a test case.
>>>> The changes are available for review under:
>>>> http://cr.openjdk.java.net/~pnarayanan/8188083/webrev.01/
>>>
>>> This version of the test is always passed before the fix(I have checked w/ and w/o jtreg).
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.