RR(XS): 7154963 crash in JvmtiEnvBase::get_current_contended_monitor()

[David Holmes]

Hi Sergey,

On 12/09/2013 8:34 PM, Sergey Gabdurakhmanov wrote:
> Hello,
>
> Here is webrev of changes I'm about to integrate:
>
> webrev: http://cr.openjdk.java.net/~sgabdura/7154963/webrev.00/
> bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154963
>
> The fix check that the _java_thread parameter is valid when it is
> possible that the JavaThread has exited after the initial checks were
> made in generated/jvmtifiles/jvmtiEnter.cpp:
> jvmti_GetCurrentContendedMonitor().
>
> The fix checked by customer on JDK6. Crash is not reproducible with this
> fix.

I don't think this fix is safe. If the thread has terminated and been 
deallocated then the code:

_java_thread->threadObj() != NULL && !_java_thread->is_exiting()

may be using a pointer that no longer points to a valid thread object. 
(This is unfortunately true in a number of places in the VM.)

Also threadObj() is never set to NULL after the thread is initialized so 
that condition seems unnecessary if the thread was already determined to 
be alive when the initial checks were made.

I would think it sufficient to only check 
Threads::includes(_java_thread) - as if that is true then the thread is 
alive and while it may be terminating it can't complete that action as 
the Threads_lock is needed for Threads::remove(this), and we hold the 
Threads_lock as this is a VM op and we are at a safepoint.

That all said, after reading the bug report (again) I'm unclear what 
role this thread is actually playing in getCurrentContendedMonitor. If 
it is not the current thread, what is it? The owner of the monitor which 
we consider contended?

David
-----

> BR,
> Sergey