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_mehr...@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-boun...@openjdk.java.net> on behalf of Volodin, > Vladislav <vladislav.volo...@sap.com> > Sent: Monday, January 27, 2020 11:11 AM > To: Sergey Bylokhov > Cc: swing-dev@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.bylok...@oracle.com> > Sent: Dienstag, 21. Januar 2020 22:26 > To: Volodin, Vladislav <vladislav.volo...@sap.com> > Cc: Jason Mehrens <jason_mehr...@hotmail.com>; swing-dev@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.bylok...@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.bylok...@oracle.com> >>>> Sent: Sunday, January 19, 2020 9:31 PM >>>> To: Volodin, Vladislav; Jason Mehrens >>>> Cc: swing-dev@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.