[jdk11u-dev] RFR: 8315135: Memory leak in the native implementation of Pack200.Unpacker.unpack() [v2]

[Thomas Stuefe]

On Tue, 29 Aug 2023 14:10:03 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> This issue was found by Yakov Shafranovich (yakovsh at amazon.com) who also provided the reproducer and proposed a fix.
>> 
>> The native implementation of the `Pack200.Unpacker` class included in OpenJDK 8 and 11 has a native and heap memory leak that gets triggered when corrupted files are processed. If the native `NativeUnpack::start()` method throws an exception (because of a corrupted input file) its caller `NativeUnpack::run()` fails to call the native `NativeUnpack::finish()` method which is responsible for freeing the allocated native memory and releasing the created global JNI handles. A Java application processing large number of malformed Pack200 files will eventually run either out of native memory or out of heap space and exit with an `OutOfMemoryError`.
>> 
>> The problem can be demonstrated with the following short test program which will exit with an `OutOfMemoryError` quite quickly if run with `java -Xmx32m NativePack200POC`:
>> 
>> 
>> import java.io.*;
>> import java.util.jar.*;
>> 
>> @SuppressWarnings("removal")
>> public class NativePack200POC {
>>   public static void main(String[] args) {
>>     try {
>>       ByteArrayInputStream in = new ByteArrayInputStream("foobar".getBytes());
>>       for(int i=0; i < 1_000_000; i++) {
>>         try {
>>           JarOutputStream out = new JarOutputStream(new ByteArrayOutputStream());
>>           Pack200.Unpacker unpacker = Pack200.newUnpacker();
>>           unpacker.unpack(in, out);
>>         } catch (IOException e) {
>>         }
>>       }
>>     } catch (OutOfMemoryError e) {
>>       System.out.println(e);
>>       throw e;
>>     }
>>   }
>> }
>> 
>> 
>> The problem can be worked around by disabling the native Pack200 implementation with `-Dcom.sun.java.util.jar.pack.disable.native=true` but the default setting is `-Dcom.sun.java.util.jar.pack.disable.native=false`.
>> 
>> Notice that this bug can not be fixed in HEAD because the Pack200 functionality has been removed in JDK 14 (https://openjdk.org/jeps/367). I therefore propose to fix this in jdk11u-dev first and then downport the fix to jdk8u-dev as well.
>> 
>
> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed indentation in test

src/java.base/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java line 123:

> 121:                        // Free up native memory and JNI handles to prevent leaks
> 122:                        ((NativeUnpack) _nunp).finish();
> 123:                     }

What happens if `NativeUnpack` already called `finish()`? In case an error happened in the window after it calling `finish()` and it returning? Would that be harmless?

-------------

PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2100#discussion_r1308948577