Based on what Ajit wrote in the bug report, he at least found a jar file
that contains this code, but I have so far failed to locate the source code
as all the versions I find on the net use the Java 2D JDK 1.2
BufferedImageOp APIs
so I suspect this bug is in a very old version and it may not be open
source.
Therefore I am not sure how much e.printStackTrace() will help them if
they don't own that
source except to encourage them to upgrade.
I'd be inclined to stick it behind a debugging property if we have one
that we could re-use
except that in such a case they probably won't know enough to set the
property.
So on balance it is probably best to do as it has been presented here
except that
catch (Throwable t) probably makes sense as we don't want to revisit
this for
a different exception.
Minor nit about the test: instead of calling intValue() on a null Integer,
why not just "throw new NullPointerException("reason ..") ?
-phil.
On 05/30/2016 01:22 AM, Ajit Ghaisas wrote:
I did contemplate catching 'Exception' instead of 'NullPointerException' while
raising the webrev, but decided not to do it as-
1. Complaint in current image filter was due to NPE only
2. Catching & consuming generic exception does not seem quite correct here as
we are just printing stack trace and not taking any concrete action.
3. There is no reported issue of any other type of Exception out of this method.
Regards,
Ajit
-----Original Message-----
From: Jim Graham
Sent: Saturday, May 28, 2016 4:41 AM
To: Ajit Ghaisas; Sergey Bylokhov; 2d-dev; Philip Race
Subject: Re: [OpenJDK 2D-Dev] Fix for JDK-8139192 : Custom ImageFilters return
blank images in Java 8(.45) while working in 7
That looks good, but I wonder if it should catch all exceptions instead of just
NPE. Historically we were only catching NPE before we added the call to
STATICIMAGEDONE, and the current filter in question turns out to throw an NPE,
but if a filter that was commonly used with offscreen images was not expecting
the STATICIMAGEDONE as was this WaterFilter, then they could potentially throw
a wider variety of exceptions.
I'm good with NPE since that is the only case we've seen, but I'd also be good
with changing line 192 to catch all exceptions. Phil?
...jim
On 5/27/16 1:32 AM, Ajit Ghaisas wrote:
Hi Jim,
Thanks for in-depth analysis and detailed review.
I appreciate your concern for filtered image being blank in Java 8, while
it used to work in Java 7.
Yes. I totally agree with your suggestion to do both -
1. Not send IMAGEERROR if there is exception from ImageFilter while
processing STATICIMAGE done
2. At the same time, as we are consuming the exception, show the exception
stacktrace.
With this fix the functionality of com.jhlabs.image.WaterFilter is
restored. It does not return blank images anymore.
Also, as additional information, exception details are shown as a
stacktrace.
Please review the updated webrev :
http://cr.openjdk.java.net/~aghaisas/8139192/webrev.02/
Regards,
Ajit
-----Original Message-----
From: Jim Graham
Sent: Thursday, May 26, 2016 4:57 AM
To: Sergey Bylokhov; Ajit Ghaisas; 2d-dev; Philip Race
Subject: Re: [OpenJDK 2D-Dev] Fix for JDK-8139192 : Custom
ImageFilters return blank images in Java 8(.45) while working in 7
Their complaint is that the resulting image is empty - most likely because the
error gets passed through and clears the image buffer. It appears that the
sequence of events is:
We send SINGLEFRAMEDONE.
- the image is displayable
We send STATICIMAGEDONE.
- filter throws NPE
- image is probably still displayable
We send ERROR
- if it gets passed through to the toolkit image consumer then we'll
clear the image buffer
- image is no longer displayable
It's hard to say for sure without having an instance of their filter in-house
for testing, but previously we weren't sending the STATICDONE notice and the
program was working just fine. Since the NPE comes from their code when we
send it, it shouldn't have affected any down-stream consumers, so we should be
OK with just continuing on and the image should still be displayable just as if
we hadn't sent the STATICDONE notice in the first place.
Now, *during debugging*, they were thwarted by the fact that we ate the
exception, true, but the original issue is that the image wasn't displayable at
all. We might want to do both:
- not send ERROR for STATICDONE when we used to not send STATICDONE at
all
- show the exception so that someone realizes that there is a problem, even
though we've made it not hurt their functionality.
Does that make sense?
...jim
On 5/25/2016 1:36 PM, Sergey Bylokhov wrote:
On 25.05.16 23:33, Jim Graham wrote:
How about option 3 -
NPE before imageComplete sends an ERROR as it does now.
NPE during imageComplete is ignored, both for backwards
compatibility and because it is more of a "hint" than an operation
that requires action from the consumer.
I guess the case is that when we ignore this NPE(w/o any
notification) the users complains that the bug is in jdk.
...jim
On 5/25/2016 1:31 AM, Ajit Ghaisas wrote:
Hi
Bug :
https://bugs.openjdk.java.net/browse/JDK-8139192
Custom ImageFilters return blank images in Java 8(.45) while
working in 7
Root cause :
private method produce() in OffScreenImageSource.java consumes a
NullPointerException that originates from a custom ImageConsumer (a
3^rd party image library class - com.jhlabs.image.WaterFilter)
Analysis:
1. How the behavior changed between JDK7 and JK8 :
A call to imageComplete(ImageConsumer.SINGLEFRAMEDONE) was added in
addition to imageComplete(ImageConsumer. STATICIMAGEDONE) as a fix
for JDK-7143612.
2. What debugging revealed:
A NullPointerException is being thrown from the library during the
call to imageComplete(ImageConsumer.STATICIMAGEDONE)
3. It looks like the fix of JDK-7143612 is valid and successive
calls to
imageComplete() are allowed.
4. The 3rd party library behavior appears incorrect (if it can not
handle subsequent calls to imageComplete(), it should de-register
itself).
The 3rd-party library code should be fixed.
Possible modifications in JDK :
Currently, OffScreenImageSource::produce() consumes the
NullPointerException - this is incorrect and results in silent
failure of this particular image filter.
We need to let the image filter library know that exception has
occurred in its code and not in JDK. We have two options -
Option 1 : Rethrow the NullPointerException --- It is the clearest way
to let 3^rd party library know that their code is erroneous with
latest JDK. This will change the 3^rd party image filter behavior
that generates blank image.
Option 2 : Add a stack trace where the exception is being consumed
--- Adding stack trace provides more information regarding failure
of 3^rd party image filter with retaining the current behavior that
generates blank image.
I have implemented both the options:
Option 1: http://cr.openjdk.java.net/~aghaisas/8139192/webrev.00/
Option 2: http://cr.openjdk.java.net/~aghaisas/8139192/webrev.01/
Request you to review both the webrevs and provide your preference.
I will add a test to the selected webrev.
Regards,
Ajit
