7186946: Refine unpacker resource usage
authorksrini
Tue, 16 Oct 2012 12:38:29 -0700
changeset 16076 d7183f4305e5
parent 16075 32c3bc19bba7
child 16077 92008ab562e0
7186946: Refine unpacker resource usage Reviewed-by: jrose, jjh, mschoene
jdk/src/share/classes/com/sun/java/util/jar/pack/NativeUnpack.java
jdk/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java
jdk/src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java
jdk/src/share/native/com/sun/java/util/jar/pack/jni.cpp
jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp
--- a/jdk/src/share/classes/com/sun/java/util/jar/pack/NativeUnpack.java	Tue Oct 16 12:35:22 2012 -0700
+++ b/jdk/src/share/classes/com/sun/java/util/jar/pack/NativeUnpack.java	Tue Oct 16 12:38:29 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/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
@@ -109,6 +109,10 @@
         return (p200 == null)? null: p200._nunp;
     }
 
+    private synchronized long getUnpackerPtr() {
+        return unpackerPtr;
+    }
+
     // Callback from the unpacker engine to get more data.
     private long readInputFn(ByteBuffer pbuf, long minlen) throws IOException {
         if (in == null)  return 0;  // nothing is readable
--- a/jdk/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java	Tue Oct 16 12:35:22 2012 -0700
+++ b/jdk/src/share/classes/com/sun/java/util/jar/pack/PackerImpl.java	Tue Oct 16 12:38:29 2012 -0700
@@ -83,7 +83,7 @@
      * @param out an OutputStream
      * @exception IOException if an error is encountered.
      */
-    public void pack(JarFile in, OutputStream out) throws IOException {
+    public synchronized void pack(JarFile in, OutputStream out) throws IOException {
         assert(Utils.currentInstance.get() == null);
         TimeZone tz = (props.getBoolean(Utils.PACK_DEFAULT_TIMEZONE))
                       ? null
@@ -118,7 +118,7 @@
      * @param out an OutputStream
      * @exception IOException if an error is encountered.
      */
-    public void pack(JarInputStream in, OutputStream out) throws IOException {
+    public synchronized void pack(JarInputStream in, OutputStream out) throws IOException {
         assert(Utils.currentInstance.get() == null);
         TimeZone tz = (props.getBoolean(Utils.PACK_DEFAULT_TIMEZONE)) ? null :
             TimeZone.getDefault();
--- a/jdk/src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java	Tue Oct 16 12:35:22 2012 -0700
+++ b/jdk/src/share/classes/com/sun/java/util/jar/pack/UnpackerImpl.java	Tue Oct 16 12:38:29 2012 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2012, Oracle and/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
@@ -106,7 +106,7 @@
      * @param out a JarOutputStream.
      * @exception IOException if an error is encountered.
      */
-    public void unpack(InputStream in, JarOutputStream out) throws IOException {
+    public synchronized void unpack(InputStream in, JarOutputStream out) throws IOException {
         if (in == null) {
             throw new NullPointerException("null input");
         }
@@ -151,7 +151,7 @@
      * @param out a JarOutputStream.
      * @exception IOException if an error is encountered.
      */
-    public void unpack(File in, JarOutputStream out) throws IOException {
+    public synchronized void unpack(File in, JarOutputStream out) throws IOException {
         if (in == null) {
             throw new NullPointerException("null input");
         }
--- a/jdk/src/share/native/com/sun/java/util/jar/pack/jni.cpp	Tue Oct 16 12:35:22 2012 -0700
+++ b/jdk/src/share/native/com/sun/java/util/jar/pack/jni.cpp	Tue Oct 16 12:38:29 2012 -0700
@@ -50,6 +50,7 @@
 static jmethodID currentInstMID;
 static jmethodID readInputMID;
 static jclass    NIclazz;
+static jmethodID getUnpackerPtrMID;
 
 static char* dbg = null;
 
@@ -60,8 +61,8 @@
 
 static unpacker* get_unpacker(JNIEnv *env, jobject pObj, bool noCreate=false) {
   unpacker* uPtr;
-  uPtr = (unpacker*)jlong2ptr(env->GetLongField(pObj, unpackerPtrFID));
-  //fprintf(stderr, "get_unpacker(%p) uPtr=%p\n", pObj, uPtr);
+  jlong p = env->CallLongMethod(pObj, getUnpackerPtrMID);
+  uPtr = (unpacker*)jlong2ptr(p);
   if (uPtr == null) {
     if (noCreate)  return null;
     uPtr = new unpacker();
@@ -94,11 +95,15 @@
   if (env == null)
     return null;
   jobject pObj = env->CallStaticObjectMethod(NIclazz, currentInstMID);
-  //fprintf(stderr, "get_unpacker() pObj=%p\n", pObj);
-  if (pObj == null)
-    return null;
-  // Got pObj and env; now do it the easy way.
-  return get_unpacker(env, pObj);
+  //fprintf(stderr, "get_unpacker0() pObj=%p\n", pObj);
+  if (pObj != null) {
+    // Got pObj and env; now do it the easy way.
+    return get_unpacker(env, pObj);
+  }
+  // this should really not happen, if it does something is seriously
+  // wrong throw an exception
+  THROW_IOE(ERROR_INTERNAL);
+  return null;
 }
 
 static void free_unpacker(JNIEnv *env, jobject pObj, unpacker* uPtr) {
@@ -137,10 +142,13 @@
                                           "()Ljava/lang/Object;");
   readInputMID = env->GetMethodID(clazz, "readInputFn",
                                   "(Ljava/nio/ByteBuffer;J)J");
+  getUnpackerPtrMID = env->GetMethodID(clazz, "getUnpackerPtr", "()J");
+
   if (unpackerPtrFID == null ||
       currentInstMID == null ||
       readInputMID == null ||
-      NIclazz == null) {
+      NIclazz == null ||
+      getUnpackerPtrMID == null) {
     THROW_IOE("cannot init class members");
   }
 }
@@ -148,8 +156,13 @@
 JNIEXPORT jlong JNICALL
 Java_com_sun_java_util_jar_pack_NativeUnpack_start(JNIEnv *env, jobject pObj,
                                    jobject pBuf, jlong offset) {
-  unpacker* uPtr = get_unpacker(env, pObj);
-
+  // try to get the unpacker pointer the hard way first, we do this to ensure
+  // valid object pointers and env is intact, if not now is good time to bail.
+  unpacker* uPtr = get_unpacker();
+  //fprintf(stderr, "start(%p) uPtr=%p initializing\n", pObj, uPtr);
+  if (uPtr == null) {
+      return -1;
+  }
   // redirect our io to the default log file or whatever.
   uPtr->redirect_stdio();
 
@@ -165,7 +178,12 @@
     else
       { buf = (char*)buf + (size_t)offset; buflen -= (size_t)offset; }
   }
-
+  // before we start off we make sure there is no other error by the time we
+  // get here
+  if (uPtr->aborting()) {
+    THROW_IOE(uPtr->get_abort_message());
+    return 0;
+  }
   uPtr->start(buf, buflen);
   if (uPtr->aborting()) {
     THROW_IOE(uPtr->get_abort_message());
--- a/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp	Tue Oct 16 12:35:22 2012 -0700
+++ b/jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp	Tue Oct 16 12:38:29 2012 -0700
@@ -3117,7 +3117,7 @@
 
 void unpacker::read_bands() {
   byte* rp0 = rp;
-
+  CHECK;
   read_file_header();
   CHECK;
 
@@ -3879,10 +3879,12 @@
 // packed file and len is the length of the buffer.
 // If null, the callback is used to fill an internal buffer.
 void unpacker::start(void* packptr, size_t len) {
+  CHECK;
   NOT_PRODUCT(debug_u = this);
   if (packptr != null && len != 0) {
     inbytes.set((byte*) packptr, len);
   }
+  CHECK;
   read_bands();
 }