Hi Pankaj,
I believe this piece of code:
793 synchronized (this) {
794 Image img = image;
795 Dimension d = adjustWidthHeight(
796 img.getWidth(imageObserver),
797 img.getHeight(imageObserver));
should look like this:
Image img
synchronized (this) {
img = image;
}
Dimension d = adjustWidthHeight(
img.getWidth(imageObserver),
img.getHeight(imageObserver));
Otherwise, a deadlock is possible when you call img.getWidth or
getHeight as I explained in my previous email.
I have removed the "headful" tag from the test as it is headless test. I added
it by mistake earlier.
I wonder whether the test runs correctly in headless environment. You're
creating a Swing component. No, you don't show it on the screen, but still…
Regards,
Alexey
On 18/11/2019 11:50, Pankaj Bansal wrote:
Hi Sergey/Alexey
<< You need to read the image to the local var under synchronization and then
get W/H from the local image variable. I guess in this case both W/H will be 0 since
error is occured?
Done. As Alexey pointed, the values are set to -1 when the error occurs.
<< I suggest clarifying the reason why image may become null:
<<794 int width = image.getWidth(imageObserver);
<<795 // Calling image.getWidth could reset image to null
if an error during loading occurred
<<796 if (image != null) {
This will not be needed now as we are using the local variable.
<<Thanks
<<The <img> tag should not be closed as in XML. I mean <img src=''/> should be
<img src=''>
<<Backlash for closing tag is not standard HTML.
<<Other than that, the test looks good.
Done. Along with this, I have removed the "headful" tag from the test as it is
headless test. I added it by mistake earlier.
I have run a mach5 job and it is green on all platforms.
Webrev: http://cr.openjdk.java.net/~pbansal/8230235/webrev02/
Regards,
Pankaj
-----Original Message-----
From: Alexey Ivanov
Sent: Wednesday, November 13, 2019 1:28 AM
To: Sergey Bylokhov; Pankaj Bansal; swing-dev
Subject: Re: <Swing Dev> [14] RFR JDK-8230235 - Rendering HTML with empty img
attribute and documentBaseKey cause Exception
On 31/10/2019 19:53, Sergey Bylokhov wrote:
On 10/31/19 5:59 am, Pankaj Bansal wrote:
<< I guess that the problem is that the "image" is accessed w/o
synchronization on "this" like in other places, but it will be
necessary to add one null check before usage of "image".
I have updated the code to access image with synchronization. But
that does not solve the issue. So I have kept the null check also. I
tried to add the null check before calling getWidth, calling getWidth
still makes the image null. So the check before calling getHeight is
still required.
You need to read the image to the local var under synchronization and
then get W/H from the local image variable.
I guess in this case both W/H will be 0 since error is occured?
You're right! This approach seems to be easier to understand.
I think the current code can deadlock: we call image.getWidth() holding the
monitor of ImageView.this; ImageObserver (line 958) needs to lock the monitor
of ImageView.this to reset 'image' field to null if an error occurred.
If an error occurs, width and height are set to -1.
--
Regards,
Alexey
