review for 6996747: SIGSEGV in nmethod::cleanup_inline_caches / CompiledIC::verify
Tom Rodriguez
tom.rodriguez at oracle.com
Thu May 12 15:55:05 PDT 2011
So I just discovered something disturbing which is that Thread::oops_do isn't virtual even though it's overridden in JavaThread. Currently it seems to work ok because they are normally called on JavaThread typed variables but if we ever called oops_do on a JavaThread that was statically typed as Thread then we'd skip the JavaThread::oops_do logic. It doesn't appear that we ever do this but it was keeping my fix from working as I intended. I've fixed that and updated my webrev.
tom
On May 12, 2011, at 3:22 PM, Tom Rodriguez wrote:
> Indeed I did. I was going to put asserts into set_scanned_nmethod to make sure it only toggled between NULL and non-null but it didn't seem necessary. Actually I just discovered a problem with this change. Hold off on reviews for a bit.
>
> tom
>
> On May 12, 2011, at 3:17 PM, Igor Veresov wrote:
>
>> 307 NMethodMarker() {
>> 308 _thread->set_scanned_nmethod(NULL);
>> 309 }
>>
>> ^^ did you mean this to be a destructor?
>>
>> Otherwise looks good.
>>
>> igor
>>
>> On 5/12/11 2:31 PM, 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::cleanup_inline_caches / 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 CompiledIC_lock, InlineCacheBuffer_lock and VtableStubs_lock
>>> 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