8151901: test/tools/pack200/Pack200Test fails on verifying native unpacked JAR

Reviewed-by: jrose

jdk/src/java.base/share/classes/com/sun/java/util/jar/pack/Package.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -476,7 +476,8 @@ class Package {
             } else if (localICs.isEmpty()) {
                 // It was a non-empty diff, but the local ICs were absent.
                 actualICs = null;
-                changed = 0;  // [] => null, no tuple change
+                // [] => null, no tuple change, but attribute deletion.
+                changed = -1;
             } else {
                 // Non-trivial diff was transmitted.
                 actualICs = computeICdiff();

jdk/src/java.base/share/classes/com/sun/java/util/jar/pack/PackageReader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -1193,18 +1193,21 @@ class PackageReader extends BandStructure {
         cls.visitRefs(VRM_CLASSIC, cpRefs);
 
         ArrayList<BootstrapMethodEntry> bsms = new ArrayList<>();
-        /*
-         * BootstrapMethod(BSMs) are added here before InnerClasses(ICs),
-         * so as to ensure the order. Noting that the BSMs  may be
-         * removed if they are not found in the CP, after the ICs expansion.
-         */
-        cls.addAttribute(Package.attrBootstrapMethodsEmpty.canonicalInstance());
-
         // flesh out the local constant pool
         ConstantPool.completeReferencesIn(cpRefs, true, bsms);
 
+        // add the bsm and references as required
+        if (!bsms.isEmpty()) {
+            cls.addAttribute(Package.attrBootstrapMethodsEmpty.canonicalInstance());
+            cpRefs.add(Package.getRefString("BootstrapMethods"));
+            Collections.sort(bsms);
+            cls.setBootstrapMethods(bsms);
+        }
+
         // Now that we know all our local class references,
         // compute the InnerClasses attribute.
+        // An InnerClasses attribute usually gets added here,
+        // although it might already have been present.
         int changed = cls.expandLocalICs();
 
         if (changed != 0) {
@@ -1221,16 +1224,6 @@ class PackageReader extends BandStructure {
             ConstantPool.completeReferencesIn(cpRefs, true, bsms);
         }
 
-        // remove the attr previously set, otherwise add the bsm and
-        // references as required
-        if (bsms.isEmpty()) {
-            cls.attributes.remove(Package.attrBootstrapMethodsEmpty.canonicalInstance());
-        } else {
-            cpRefs.add(Package.getRefString("BootstrapMethods"));
-            Collections.sort(bsms);
-            cls.setBootstrapMethods(bsms);
-        }
-
         // construct a local constant pool
         int numDoubles = 0;
         for (Entry e : cpRefs) {

jdk/test/ProblemList.txt

@@ -310,8 +310,6 @@ sanity/client/SwingSet/src/ButtonDemoScreenshotTest.java        8157338 generic-
 
 tools/pack200/CommandLineTests.java                             7143279,8059906 generic-all
 
-tools/pack200/Pack200Test.java                                  8059906,8151901 generic-all
-
 tools/launcher/FXLauncherTest.java                              8068049 linux-all,macosx-all
 
 tools/pack200/Pack200Props.java                                 8155857 generic-all

jdk/test/tools/pack200/Pack200Test.java

@@ -21,6 +21,14 @@
  * questions.
  */
 
+ /*
+  * @test
+  * @bug 6521334 6712743 8007902 8151901
+  * @summary test general packer/unpacker functionality
+  *          using native and java unpackers
+  * @compile -XDignore.symbol.file Utils.java Pack200Test.java
+  * @run main/othervm/timeout=1200 -Xmx1280m -Xshare:off Pack200Test
+  */
 
 import java.util.*;
 import java.io.*;
@@ -28,17 +36,6 @@ import java.lang.management.ManagementFactory;
 import java.lang.management.MemoryMXBean;
 import java.util.jar.*;
 
- /*
-  * @test
-  * @bug 6521334 6712743 8007902
-  * @key intermittent
-  * @summary check for memory leaks, test general packer/unpacker functionality\
-  *          using native and java unpackers
-  * @compile -XDignore.symbol.file Utils.java Pack200Test.java
-  * @run main/othervm/timeout=1200 -Xmx1280m -Xshare:off Pack200Test
-  * @author ksrini
-  */
-
 /**
  * Tests the packing/unpacking via the APIs.
  */
@@ -48,6 +45,9 @@ public class Pack200Test {
     static final MemoryMXBean mmxbean = ManagementFactory.getMemoryMXBean();
     static final long m0 = getUsedMemory();
     static final int LEAK_TOLERANCE = 21000; // OS and GC related variations.
+    // enable leak checks only if required, GC charecteristics vary on
+    // platforms and this may not yield consistent results
+    static final boolean LEAK_CHECK = Boolean.getBoolean("Pack200Test.enableLeakCheck");
 
     /** Creates a new instance of Pack200Test */
     private Pack200Test() {}
@@ -60,6 +60,8 @@ public class Pack200Test {
     }
 
     private static void leakCheck() throws Exception {
+        if (!LEAK_CHECK)
+            return;
         long diff = getUsedMemory() - m0;
         System.out.println("  Info: memory diff = " + diff + "K");
         if (diff > LEAK_TOLERANCE) {
@@ -126,7 +128,7 @@ public class Pack200Test {
     /**
      * @param args the command line arguments
      */
-    public static void main(String[] args) throws IOException {
+    public static void main(String[] args) throws Exception {
         // select the jars carefully, adding more jars will increase the
         // testing time, especially for jprt.
         jarList.add(Utils.createRtJar());