RFR (S) 8218755: [REDO] Symbol leak in prepend_host_package_name

[David Holmes]

Hi Coleen,

On 13/02/2019 1:13 am, coleen.phillimore at oracle.com wrote:
> Summary: fix Symbol refcounting again, add comment and a test.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/2019/8218755.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8218755
> 
> I hope this is better.  Ran hs-tier1-6 and a new test!

You got rid of the NULL check in update_class_name() but the one 
remaining call-site has:

   // Update _class_name which could be null previously
   // to reflect the name in the constant pool
   update_class_name(class_name_in_cp);

implying we still need it - even though we assert _class_name is not 
NULL earlier (perhaps shouldn't be NULL but we need to be conservative?).

I'm inclined to just inline update_class_name along with the comments - 
they can be shortened and are no worse than the block you added elsewhere:

!     // The new class name is created with a refcount of one. When 
installed into the InstanceKlass,
!     // it'll be two and when the ClassFileParser destructor runs, 
it'll go back to one and get deleted
!     // when the class is unloaded.

---

test/hotspot/jtreg/runtime/defineAnonClass/AnonSymbolLeak.java

24 // This is copied from these DefineAnon

delete "these"

Aside: singleoneInstanceField is an insanely long (and strange) name for 
a local variable that gets written out three times!

Aside 2: these tests should be migrated off sun.misc.Unsafe and use 
jdk.internal.misc.Unsafe

   81         try {
   82             p1cls.getMethod("test").invoke(null);
   83         } catch (Throwable ex) {
   84             ex.printStackTrace();
   85             fail = ex;  // throw this to make test fail, since 
subtest failed
   86         }

This code can never fail. If the exception occurs we store it in the 
local variable fail but don't do anything with it. ??

Thanks,
David
----

> Thanks,
> Coleen