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.
