review for 6996747: SIGSEGV in nmethod::cleanup_inline_caches / CompiledIC::verify (original) (raw)

Tom Rodriguez tom.rodriguez at oracle.com
Thu May 12 16:31:22 PDT 2011


On May 12, 2011, at 4:11 PM, Vladimir Kozlov wrote:

> It's a static so it's automatically initialized to 0.

Agree, but explicit initialization to 0 is much better to understand.

Ok.

nm->compilekind() is not char so using %c is wrong.

Oops. My brain saw char and ran with it. I switch to %s and added check for NULL.

Why reportevents(int id, address entry) uses ARRAYSIZE(records) as limit?

Leftovers again.

reportevents() misses the check for empty records. May be do memset to 0 after creating the buffer and then check for 0 in reportevents.

Good idea.

tom

Vladimir Tom Rodriguez wrote: On May 12, 2011, at 3:41 PM, Vladimir Kozlov wrote:

Memory leak since possiblysweep() could be called several time. You need to free or reset previous ring buffer. I converted from a static buffer to a malloc'ed one and forgot to add the guard so it's only init'ed once. Missing sweepindex initialization. It's a static so it's automatically initialized to 0. SweeperRecord::kind is not printed. I added that. Should it be assert/guarantee? :

+ if (m != nm->method()) { + tty->printcr("method oop changed during lock acquire: " INTPTRFORMAT " != " INTPTRFORMAT, m, nm->method()); + } This should have been deleted. It was debug code I used to confirm the bug. tom Vlaidmir Tom Rodriguez wrote: http://cr.openjdk.java.net/~never/6996747 149 lines changed: 149 ins; 0 del; 0 mod; 10537 unchg 6996747: SIGSEGV in nmethod::cleanupinlinecaches / CompiledIC::verify Reviewed-by: When the sweeper is processing an nmethod it's possible for a safepoint to occur while acquiring locks to clean the inline caches. This can allow the nmethod to be unloaded in the middle of processing it which can result in assertion failures or crashes. I considered modifying the locks to skip the safepoint check but it would require changing CompiledIClock, InlineCacheBufferlock and VtableStubslock which seems risky. Instead I keep track of the currently nmethod in the CompiledThread and scan it when a GC occurs. I also included some sweeper logging code that I wrote while debugging this. Tested with failing test from report though we'll need big apps runs to confirm that there aren't other issues.



More information about the hotspot-compiler-dev mailing list