<Swing Dev> [14] RFR JDK-8230235 - Rendering HTML with empty img attribute and documentBaseKey cause Exception
Alexey Ivanov
alexey.ivanov at oracle.com
Mon Nov 18 12:08:26 UTC 2019
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
More information about the swing-dev
mailing list