JDK-8191017 : NullPointerExcpn-java.awt.image.FilteredImageSource.startProduction JDK-8079607
  • Type: CSR
  • Component: client-libs
  • Sub-Component: java.awt
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 10
  • Submitted: 2017-11-09
  • Updated: 2017-12-12
  • Resolved: 2017-12-12
Related Reports
CSR :  
JDK-8188083 - NullPointerExcpn-java.awt.image.FilteredImageSource.startProduction JDK-8079607
Description
Summary
-------

Synchronize the methods of FilteredImageSource to avoid NullPointerExceptions while accessing the HashTable.

Problem
-------

A NullPointerException was observed at java.awt.image.FilteredImageSource.startProduction method.

FilteredImageSource instantiates a HashTable for its operation. The methods- startProduction and requestTopDownLeftRightResend of FilteredImageSource read or update this HashTable in un-synchronized manner. In a multi-threaded environment, the un-synchronized access to the HashTable is susceptible to the exceptions as reported in the bug.

Solution
--------

The proposed solution makes the concerned methods 'synchronized' after duly evaluating possibilities for deadlocks.

Specification
-------------
<code>
&nbsp;&nbsp;/** <br>
&nbsp;&nbsp;&nbsp;* This class is an implementation of the ImageProducer interface which<br>
&nbsp;&nbsp;&nbsp;* takes an existing image and a filter object and uses them to produce<br>
-- * image data for a new filtered version of the original image.<br>
++ * image data for a new filtered version of the original image. Furthermore,<br>
++ * {@code FilteredImageSource} is safe for use by multiple threads.<br>
<br>
--    public void startProduction(ImageConsumer ic) { <br>
++    public synchronized void startProduction(ImageConsumer ic) { <br>

--    public void requestTopDownLeftRightResend(ImageConsumer ic) { <br>
++    public synchronized void requestTopDownLeftRightResend(ImageConsumer ic) { <br>
</code>
Comments
Voting to approve the amended request.
12-12-2017
I think the point is that if adding synchronized does not change the signature, and nor does it change the thread safety contract of this class - documented already or not - then why do we need to document it now. And so maybe we don't need a CSR after all ?
29-11-2017
The contract to be mentioned, or not, is not whether or not a method has the "synchronized" modifier. This is for a number of reasons, including the equivalence of these two ways of implementing method foo: synchronized void foo() { // Logic of method } void foo() { synchronized(this) { // Logic of method } } See discussion in section 8.4.3.6 of the Java Language Specification. The specified behavior should discuss the thread-safely of the classes in question.
29-11-2017
[~darcy] Thank you for your suggestion. Just a minor clarification: <br> . The existing code at FilteredImageSource and many API classes in java.awt.image package contain 'synchronized' methods but none of these methods explicitly mention the synchronized contract in the javadoc comments. <br> . In view to this, kindly suggest if the explicit mentioning of 'synchronized' contract is required for this change.
16-11-2017
Technically adding synchronized is not altering the specification of the methods in question. The presence of the synchronized modifiers is not reflected in the javadoc output. However, in concert with adding the synchronized the specification should be updated to explicitly state the concurrency contract the class.
10-11-2017
. The link mentioned below leads to the code review thread containing discussion points and data validating the fix proposed in this CSR. <br> . http://mail.openjdk.java.net/pipermail/2d-dev/2017-November/008677.html
10-11-2017
I have reviewed this but update the compatibility section to discuss at least the potential for deadlocks even if we believe it has been properly tested for such a possibility
09-11-2017