On Wed, 5 Mar 2025 22:38:17 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>>> But this issue should be addressed—it's similar to a zip bomb, where small >>> but valid source data can consume a huge amount of memory after processing. >> >> Unlike zip bomb - which is a file that has a valid header but no valid data >> inside - this time the data is valid, and there are cases where very small >> amount of input data require a lot of resources to process it, this is >> exactly the case. >> >>> To handle this, we should convert a fatal OutOfMemoryError into an >>> IOException, which the application is likely already expecting. >> >> No we should not. The OutOfMemoryError is an extension of >> VirtualMachineError which says "Thrown to indicate that the Java Virtual >> Machine is broken or has run out of resources necessary for it to continue >> operating." - which means that after issuing this kind of error the >> operation of virtual machine can not be guaranteed and "GC will clean up" >> might never happen. Additionally, the VirtualMachineError is an extension of >> Error which says "An Error is a subclass of Throwable that indicates serious >> problems that a reasonable application should not try to catch." and by >> converting Error into a checked exception we are doing just that - we hide >> the underlying problem and making user application catch VM error as if >> nothing happened in the background. That is just wrong. We can handle it >> much better. For example i can add a threshold to check that the memory >> requirement does not exceed 90% of the available memory - that will take >> care of the overhead structures and buffers needed to pr ocess the sound bank. But if program is made that it allows concurrent processing of the sound banks that will not help either because two threads will race to exhaust heap and one of them will ultimately eat up the last byte and we will still fail. > >> No we should not. The OutOfMemoryError is an extension of >> VirtualMachineError which says "Thrown to indicate that the Java Virtual >> Machine is broken or has run out of resources necessary for it to continue >> operating." > > Then just check all usages of "catch (OutOfMemoryError e)" in the java.base > and java.desktop/sound/2d/ modules. You did not mention the doc for OOM > "Thrown when the Java Virtual Machine cannot allocate an object because it is > out of memory, **and no more memory could be made available by the garbage > collector**." >Unlike zip bomb - which is a file that has a valid header but no valid data >inside - this time the data is valid, and there are cases where very small >amount of input data require a lot of resources to process it, this is exactly >the case. But is that really the case? In the test program, we have a MIDI file that we convert into an audio stream using the default sound bank from the JDK. Then, we try convert the audio stream back into a sound bank. What should be expected from MidiSystem.getSoundbank(midiStream)? I assume it should return the sound bank [used ](https://github.com/openjdk/jdk/blob/11a37c829c12d064874416a7b242596cf23972e5/src/java.desktop/share/classes/com/sun/media/sound/SoftSynthesizer.java#L355) to render the MIDI file. And if no sound bank is found, the default one should be returned? Can we extract that w/o decoding? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23814#discussion_r1982295649
