RFR: 8055461: getNextID in ImageIcon class can lead to overflow [v2]

[Alexey Ivanov]

On Wed, 11 Jun 2025 15:04:21 GMT, Prasanta Sadhukhan <psadhukhan at 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