On Wed, 11 Jun 2025 15:04:21 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>> ImageIcon.getNextID uses `mediaTrackerID ` which do not detect overflow. >> >> Theoretically there is a possibility that there can be overflow in the long >> time run or for large number of created "imageIcon" >> >> Made sure there is no overflow and treat that loadImage as ABORTED >> >> No regression testcase as it addresses theoretical possibility.. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Reset mediaTrackerID if it overflows Here's the test case that I used. (Thanks to @mickleness for the code, setting an image significantly increased the performance of the test; my initial test loaded an image from the disk for each instance of `ImageIcon`, and it took more than 6 hours to overflow `mediaTrackerId` into negative values.) <details> <summary><code>ImageIconOverflow.java</code></summary> import java.awt.Graphics; import java.awt.Image; import java.awt.Toolkit; import java.awt.image.BufferedImage; import java.io.File; import java.io.IOException; import java.util.stream.IntStream; import javax.imageio.ImageIO; import javax.swing.ImageIcon; import static java.awt.image.BufferedImage.TYPE_INT_RGB; public class ImageIconOverflow { private static final String IMAGE_FILENAME = "image.png"; public static void main(String[] args) throws IOException { createImage(); final Image image = Toolkit.getDefaultToolkit().createImage("onepixel.gif"); IntStream.range(0, Integer.MAX_VALUE - 1) .forEach((c) -> new ImageIcon(image)); System.out.println("---"); new ImageIcon(IMAGE_FILENAME); System.out.println("+++"); new ImageIcon(IMAGE_FILENAME); new ImageIcon(IMAGE_FILENAME); System.out.println("###"); IntStream.range(0, Integer.MAX_VALUE - 1) .forEach((c) -> new ImageIcon(image)); System.out.println("---"); new ImageIcon(IMAGE_FILENAME); System.out.println("+++"); new ImageIcon(IMAGE_FILENAME); new ImageIcon(IMAGE_FILENAME); } private static void createImage() throws IOException { BufferedImage image = new BufferedImage(1, 1, TYPE_INT_RGB); Graphics g = image.getGraphics(); g.fillRect(0, 0, image.getWidth(), image.getHeight()); g.dispose(); File file = new File(IMAGE_FILENAME); ImageIO.write(image, "png", file); file.deleteOnExit(); } } </details> I also modified `IconImage.java` to print the id. <details> <summary>Diff for <code>ImageIcon.java</code></summary> diff --git a/src/java.desktop/share/classes/javax/swing/ImageIcon.java b/src/java.desktop/share/classes/javax/swing/ImageIcon.java index 2bae31ba31f..8638da90567 100644 --- a/src/java.desktop/share/classes/javax/swing/ImageIcon.java +++ b/src/java.desktop/share/classes/javax/swing/ImageIcon.java @@ -296,6 +296,13 @@ protected void loadImage(Image image) { synchronized(mTracker) { int id = getNextID(); + if (id <= 0x8000000F + || (id >= 0 && id <= 0x0000000F) + || id > 0x7ffffff0) { + System.out.println("ImageIcon.id=" + String.format("%08x", id) + + " " + id); + } + mTracker.addImage(image, id); try { mTracker.waitForID(id, 0); </details> In 25 minutes, the case overflowed twice: from positive into negative, and then from negative into positive. ImageIcon.id=00000001 1 ImageIcon.id=00000002 2 ... ImageIcon.id=7ffffffd 2147483645 ImageIcon.id=7ffffffe 2147483646 --- ImageIcon.id=7fffffff 2147483647 +++ ImageIcon.id=80000000 -2147483648 ImageIcon.id=80000001 -2147483647 ### ImageIcon.id=80000002 -2147483646 ... ImageIcon.id=8000000f -2147483633 --- ImageIcon.id=00000000 0 +++ ImageIcon.id=00000001 1 ImageIcon.id=00000002 2 Thus, I think there's nothing to fix. Yes, `IconImage.mediaTrackerId` can overflow, but it's not an issue. Therefore, I propose to close the bug as *‘Not an Issue’* or *‘Won't Fix’*. Prasanta's second [commit fd2ad26](https://github.com/openjdk/jdk/pull/25666/commits/fd2ad261356ec85585fe2f838ccc1273dd90b390) resolves the problem with overflow the way that [I suggested](https://github.com/openjdk/jdk/pull/25666#issuecomment-2962726712). But it fixes the potential problem that doesn't affect the behaviour of `ImageIcon` that is proved by the test in this comment. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25666#issuecomment-2967332196