RFR: 8055461: getNextID in ImageIcon class can lead to overflow
Alexey Ivanov
aivanov at openjdk.org
Tue Jun 10 13:14:32 UTC 2025
On Tue, 10 Jun 2025 07:27:41 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:
>>> What condition?
>>
>> I just meant the int overflow condition. That test is not meant to demonstrate an OOM.
>>
>> I understand why an OOM feels related to this ticket (and maybe that really is the compelling concern here), but I don't think that ticket mentioned memory consumption.
>>
>> Here's a recording that (I think?) shows I reached the int overflow in about 15 minutes:
>> https://go.screenpal.com/watch/cT16h0nX6xx
>>
>> (You may need to look at the system clock in top-right to accurately track time; parts of the video are sped up 10x. It should be pretty clear when the text cursor blinks super fast.)
>>
>> We could expand that test to:
>> A. use an animated gif, or
>> B. try to load a new non-trivial image once we cross into a negative ID
>>
>> ... but that circles back around to the broader "what are we really trying to solve" question. (And I don't think I know the answer to that question.)
>>
>>> This issue was not raised by me so clearly somebody felt the need for it.
>>
>> Sure.
>>
>>> Also, I think it is better to get ABORTED status for the image to let the user know so that corrective action can be taken, instead of aborting the application with OOM or whatever exception it gets thrown
>>
>> OK. This feels too theoretical for me to invest strongly in a recommendation.
>>
>> On the one hand: what you're saying makes sense, and obviously avoiding an OOM is a good thing to do.
>>
>> On the other hand: if the integer overflow occurs in a non-trivial real-world usage: I'm guessing the images/icons already *are* actually being responsibly disposed of/garbage collected over time. So what we really need is to reset the counter or use a new MediaTracker if the int overflow condition is reached. For example, suppose we use all 2147483647 images IDs, and each image is 100x100px and 3 color channels. That's going to require about 60,000 GB of data (in terms of pixels). If those images are kept in memory, then we will reach an OOM long before we reach an integer overflow. So the fact that we get here (to the integer overflow) at all may (?) mean the application is responsibly discarding images and it's able to handle thousands more over time.
>>
>> Most of all I'd like to see a real-world use case explaining how this impacts a user. Until then: this PR looks good/harmless (IMO).
>
>> OK. This feels too theoretical for me to invest strongly in a recommendation.
>
> That's why I mentioned in the description itself
>
>> Theoretically there is a possibility that there can be overflow in the long time run or for large number of created "imageIcon"
What's the deal with OOM at all?
If the application doesn't keep all the images it loads, no OOM will occur. Throwing OOM has nothing to do with the stated problem.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25666#discussion_r2137862532
More information about the client-libs-dev
mailing list