<Swing Dev> Remove System.out.println from ImageIcon.loadImage
Volodin, Vladislav
vladislav.volodin at sap.com
Tue Jan 28 20:54:03 UTC 2020
Hi Jason,
I am not sure about the loop, because I don’t know if we can trust results from mTracker.statusID at the moment when an exceptional case occurs. For example, I was experimenting with the code and found out that the MediaTracker can spawn a thread to load the image, or might use the current thread, and the ABORTED state is never returned (I don’t even know how to raise this state). That is why I don’t even know if your loop will ever stop.
In my solution, I give the second chance only, and in case if the execution was interrupted the second time, I explicitly assign the ABORTED state to loadStatus. If the user wants to load this image once again, he can create ImageIcon again (or even do this in a loop), or reinitialize it with a single method (I don’t remember its name).
What do you think?
Kind regards,
Vlad
Sent from myPad
> On 28. Jan 2020, at 21:34, Jason Mehrens <jason_mehrens at hotmail.com> wrote:
>
> Vlad,
>
> I assume you would want to wait in a loop and reassert the interrupt at the end.
>
> ===
> protected void loadImage(Image image) {
> MediaTracker mTracker = getTracker();
> synchronized(mTracker) {
> int id = getNextID();
>
> mTracker.addImage(image, id);
> boolean wasInterrupted = false;
> try {
> do {
> try {
> mTracker.waitForID(id, 0);
> } catch (InterruptedException e) {
> wasInterrupted = true;
> }
> loadStatus = mTracker.statusID(id, false);
> } while (loadStatus == MediaTracker.LOADING);
> mTracker.removeImage(image, id);
>
> width = image.getWidth(imageObserver);
> height = image.getHeight(imageObserver);
> } finally {
> if (wasInterrupted) {
> Thread.currentThread().interrupt();
> }
> }
> }
> }
> ===
>
> Jason
> ________________________________________
> From: swing-dev <swing-dev-bounces at openjdk.java.net> on behalf of Volodin, Vladislav <vladislav.volodin at sap.com>
> Sent: Monday, January 27, 2020 11:11 AM
> To: Sergey Bylokhov
> Cc: swing-dev at openjdk.java.net
> Subject: Re: <Swing Dev> Remove System.out.println from ImageIcon.loadImage
>
> Hello Sergey, and others,
>
> here is the patch (made with "git diff") in the attachment. I have read that sometimes the attachments are lost. So here is my version with few comments regarding my code. I decided to use your approach with a small improvement. There is an excerpt of my patch.
>
> The idea is pretty simple:
> 1. I create an empty gif 1x1;
> 2. Initialize DefaultToolkit (I plan to interrupt the main thread, and if I do not initialize the toolkit in advance, my test will fail at the beginning
> 3. Interrupt the main thread
> 4. Try to load the icon:
>
> import javax.swing.*;
> import java.awt.*;
>
> public class imageIconInterrupted {
> static byte[] EMPTY_GIF = new byte[]{
> (byte) 0x47, (byte) 0x49, (byte) 0x46, (byte) 0x38, (byte) 0x39, (byte) 0x61, (byte) 0x01, (byte) 0x00,
> (byte) 0x01, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x21, (byte) 0xF9, (byte) 0x04,
> (byte) 0x01, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x2C, (byte) 0x00, (byte) 0x00,
> (byte) 0x00, (byte) 0x00, (byte) 0x01, (byte) 0x00, (byte) 0x01, (byte) 0x00, (byte) 0x00, (byte) 0x02
> };
>
> public static void main(String[] args) throws Exception {
> Toolkit ignoreToolkit = Toolkit.getDefaultToolkit();
>
> Thread.currentThread().interrupt();
> ImageIcon iconInterrupt = new ImageIcon(EMPTY_GIF);
> if (iconInterrupt.getImageLoadStatus() != MediaTracker.COMPLETE) {
> throw new RuntimeException("Couldn't load GIF from bytes after interruption");
> }
> }
> }
>
> The code of loadImage I have changed as follows:
> 1. I clear loadStatus for "finally" part;
> 2. Check the interruption according to your comments;
> 3. Change the status to ABORTED, if the interruption happened again (this part I cannot test, because I cannot properly interrupt the thread whenever I want. Multithreading is quite fragile there :( )
> 4. update the status "interrupted" again;
> 5. and then - finally block.
>
> protected void loadImage(Image image) {
> ...
> try {
> loadStatus = 0;
> mTracker.addImage(image, id);
> mTracker.waitForID(id, 0);
> } catch (InterruptedException e1) {
> try {
> mTracker.waitForID(id, 0);
> } catch (InterruptedException e2) {
> loadStatus = MediaTracker.ABORTED;
> Thread.currentThread().interrupt();
> }
> } finally {
> if (loadStatus == 0) {
> loadStatus = mTracker.statusID(id, false);
> }
> mTracker.removeImage(image, id);
> }
>
> P.S. if you think that my patch sounds fine, I will find a sponsor for the bug report and the patch preparation, so you can review it later.
> After the second attempt, my EMPTY_GIF was loaded successfully. The ABORTED part it seems that I don't know if it has ever worked. My patch (in the attachment) checks also the state ERROR for the URL that doesn't exist. Something like:
>
> ImageIcon iconNotExist = new ImageIcon(new URL("http://doesnt.exist.anywhere/1.gif")); // I am not sure if I spelled this URL grammatically correct
> if (iconNotExist.getImageLoadStatus() != MediaTracker.ERRORED) {
> throw new RuntimeException("Got unexpected status from non-existing GIF");
> }
>
> Thanks to everyone.
>
> Kind regards,
> Vlad
>
> -----Original Message-----
> From: Sergey Bylokhov <Sergey.Bylokhov at oracle.com>
> Sent: Dienstag, 21. Januar 2020 22:26
> To: Volodin, Vladislav <vladislav.volodin at sap.com>
> Cc: Jason Mehrens <jason_mehrens at hotmail.com>; swing-dev at openjdk.java.net
> Subject: Re: <Swing Dev> Remove System.out.println from ImageIcon.loadImage
>
>> On 1/21/20 1:14 pm, Volodin, Vladislav wrote:
>> Hi all,
>>
>> If I am not mistaken, this method is called from the constructor and other methods. How long should we try loading the icon using the unmanaged (by any thread pool, but I am not sure about this statement) thread?
>>
>> We don’t even have the flag “interrupted”. So technically this icon has to be loaded.
>
> I think it is necessary to save the "interrupted" state in the catch
> block and try to call waitForID() again. It will be necessary to set
> "interrupted" flag for the Thread after that(when the waitForID will
> return without exception).
>
>
>> Sent from myFone
>>
>>>> On 21. Jan 2020, at 21:55, Sergey Bylokhov <Sergey.Bylokhov at oracle.com> wrote:
>>>
>>> On 1/21/20 12:26 pm, Jason Mehrens wrote:
>>>> +1 for Sergey suggestion.
>>>
>>> Or probably we need to try to load the image again because of the spec of the method state this:
>>> "Loads the image, returning only when the image is loaded."
>>>
>>>> ________________________________________
>>>> From: Sergey Bylokhov <Sergey.Bylokhov at oracle.com>
>>>> Sent: Sunday, January 19, 2020 9:31 PM
>>>> To: Volodin, Vladislav; Jason Mehrens
>>>> Cc: swing-dev at openjdk.java.net
>>>> Subject: Re: <Swing Dev> Remove System.out.println from ImageIcon.loadImage
>>>> I guess there are no objections, probably the best way to fix
>>>> it is to drop the System.out.println and set interrupted flag.
>>>> On 1/16/20 7:05 am, Volodin, Vladislav wrote:
>>>>> If people in this distribution list agree, I can start working on a simple fix and either remove the logging (System.out.println), or replace it with PlatformLogger. I guess the second solution sounds better, but I don't know what is the better way to write a test-case for it.
>>>> --
>>>> Best regards, Sergey.
>>>
>>>
>>> --
>>> Best regards, Sergey.
>
>
> --
> Best regards, Sergey.
More information about the swing-dev
mailing list