[9] RFR(M): 8054292 : code comments leak in fastdebug builds

[Vladimir Kozlov]

There is some missmatch of use #ifdef ASSERT and #ifndef PRODUCT.
Please, build 'optimized' JVM in addition to 'product' and 'debug'.
Seems _defunct field should be DEBUG only since it is checked only in 
assert. It could be NOT_PRODUCT bu in that case 
set_null_and_invalidate() should clean both fields under #ifndef PRODUCT.
isNull() will return 'true' in NOT_PRODUCT (optimized VM) when assign() 
may set _strings.

An other thing  I don't like in current code is the same name of 
_strings fields in both CodeStrings and CodeBuffer. It is difficult to 
follow which one is accessed in a code.

 > One client of CodeBuffers allocates them without running their 
constructor,
 > which interfered with enabling the full set of assertions that we 
might like.

It would be nice to fix it - call constructor in that place. Where it 
is? You can try to do what we do in growableArray:
  new(&_strings) CodeStrings().

Thanks,
Vladimir

On 8/22/14 2:24 PM, David Chase wrote:
>
> webrev: http://cr.openjdk.java.net/~drchase/8054292/webrev-for-leak-checks.09/
> bug: (confidential) https://bugs.openjdk.java.net/browse/JDK-8054292
>
> description: CodeBuffers and BufferBlobs leak code comment strings in fastdebug.
> This can be seen with the KitchenSink internal test or with CompileTheWorld:
>
> w/ CodeBuffer strings freed, BufferBlobs flushed to free strings and oopmaps:
> Total: reserved=5,908,975KB, committed=713,911KB
> - Code (reserved=251,478KB, committed=22,442KB)
> (malloc=3,798KB #82,818)
> (mmap: reserved=247,680KB, committed=18,644KB)
>
> no leak plugging:
> Total: reserved=6,031,728KB, committed=836,404KB
> - Code (reserved=340,883KB, committed=111,587KB)
> (malloc=93,203KB #221,6790)
> (mmap: reserved=247,680KB, committed=18,384KB)
>
> With these leaks fixed (this includes a possible oopmap leak)
> KitchenSink still leaks about about 6MB/hour on a not-too-old
> 6MB/hour Linux box, but this leak appears to be plugged.
> (KitchenSink is running over the weekend with NMT detail
> turned on to identify the next leak.  The old code leaked
> at least 25MB/hour on this box.)
>
> Fix:
> Free memory in appropriate places.
> Insert asserts to guard against reuse of freed memory.
>
> Testing:
> JPRT, jtreg, CompileTheWorld, KitchenSink, quicktest
>
> jtreg included testing with -XX:+UnlockDiagnosticVMOptions  -XX:+PrintAssembly
> to help guard against the case of prematurely freed pointers.
>
> Other testing involved freeing more aggressively at the use sites,
> and then asserting whenever the destructor was run w/o memory
> having been freed.
>
> One client of CodeBuffers allocates them without running their constructor,
> which interfered with enabling the full set of assertions that we might like.
>