Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8315135: Memory leak in the native implementation of Pack200.Unpacker…
….unpack()

Co-authored-by: Yakov Shafranovich <yakovsh@amazon.com>
Reviewed-by: clanger, stuefe
  • Loading branch information
simonis and yakovsh committed Aug 30, 2023
1 parent b984b0a commit b77c161
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 2 deletions.
Expand Up @@ -61,7 +61,7 @@ class NativeUnpack {

// Resets the engine and frees all resources.
// Returns total number of bytes consumed by the engine.
private synchronized native long finish();
synchronized native long finish();

// Setting state in the unpacker.
protected synchronized native boolean setOption(String opt, String value);
Expand Down
Expand Up @@ -116,6 +116,11 @@ public synchronized void unpack(InputStream in, JarOutputStream out) throws IOEx
} catch (UnsatisfiedLinkError | NoClassDefFoundError ex) {
// failover to java implementation
(new DoUnpack()).run(in0, out);
} finally {
if (_nunp != null) {
// Free up native memory and JNI handles to prevent leaks
((NativeUnpack) _nunp).finish();
}
}
in0.close();
Utils.markJarFile(out);
Expand Down
5 changes: 4 additions & 1 deletion src/jdk.pack/share/native/libunpack/jni.cpp
Expand Up @@ -314,9 +314,12 @@ Java_com_sun_java_util_jar_pack_NativeUnpack_getUnusedInput(JNIEnv *env, jobject

JNIEXPORT jlong JNICALL
Java_com_sun_java_util_jar_pack_NativeUnpack_finish(JNIEnv *env, jobject pObj) {
unpacker* uPtr = get_unpacker(env, pObj, false);
// There's no need to create a new unpacker here if we don't already have one
// just to immediatly free it afterwards.
unpacker* uPtr = get_unpacker(env, pObj, /* noCreate= */ true);
CHECK_EXCEPTION_RETURN_VALUE(uPtr, 0);
size_t consumed = uPtr->input_consumed();
// free_unpacker() will set the unpacker field on 'pObj' to null
free_unpacker(env, pObj, uPtr);
return consumed;
}
Expand Down
55 changes: 55 additions & 0 deletions test/jdk/tools/pack200/UnpackMalformed.java
@@ -0,0 +1,55 @@
/*
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*
*/

/*
* @test
* @bug 8315135
* @run main/othervm/timeout=300 -Dcom.sun.java.util.jar.pack.disable.native=false -Xmx8m UnpackMalformed
*/

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.jar.JarOutputStream;
import java.util.jar.Pack200;

@SuppressWarnings("removal")
public class UnpackMalformed {
public static void main(String[] args) {
try {
ByteArrayInputStream in = new ByteArrayInputStream("foobar".getBytes());
for (int i=0; i < 1_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;
}
}
}

1 comment on commit b77c161

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.