RFR (S): 8213397: Stack dump should show more clearly when a thread is blocked on a class initialization monitor

[David Holmes]

Hi Coleen,

On 14/01/2019 10:25 pm, coleen.phillimore at oracle.com wrote:
> 
> This is a nice improvement.

Thanks for taking a look.

> http://cr.openjdk.java.net/~dholmes/8213397/webrev/src/hotspot/share/runtime/vframe.cpp.udiff.html 
> 
> 
> + // top-frame, so we should be waiting on a Class initialization monitor.
> + InstanceKlass* k = thread()->class_to_be_initialized();
> + if (k != NULL) {
> + st->print_cr("\t- waiting on the Class initialization monitor for %s", 
> k->external_name());
> + }
> + else {
> + assert(false, "OBJECT_WAIT state is unexpected");
> + }
> 
> 
> Should be } else {
> 
> And I don't think this should be an assert.  There are a couple of other 
> places that we lock an object monitor from the VM, but I don't think we 
> wait in these places.  In any case, if we add a wait, it would take a 
> lot of testing and a long time to get to this assert to find it needs to 
> add another case here.  So if you remove the 'else' completely and 
> change the comment to "see if we are waiting on a class initialization 
> monitor" you don't need to fix the {}.

Initially I thought the "else" might be some JNI or JVM TI case, but 
that's not so as there's no direct API to do wait(), it just requires 
invoking the Object.wait method. So at the moment the implicit else is 
"unreachable" and I wanted to convey that. I chose the assert in case I 
was wrong, or in the unlikely event a new VM internal call to wait() was 
ever added and we happened to encounter it. Leaving it empty to me 
raises the question "how do we get here?".

Thanks,
David

> Thanks,
> Coleen
> 
> 
> 
> On 1/14/19 1:01 AM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213397
>> webrev: http://cr.openjdk.java.net/~dholmes/8213397/webrev/
>>
>> Please review this simple enhancement to improve the stack trace 
>> information when a thread is blocked doing a (logical) Object.wait on 
>> the monitor associated with Class static initialization.
>>
>> Details are in the bug report but basically we record, in a 
>> thread-local variable, the name of the class before the "wait" and 
>> clear it after, thus allowing the stack dump code to query it if it 
>> appears we may be in that situation.
>>
>> In InstanceKlass::initialize_impl I also refactored the code a tiny 
>> bit to make use of the JavaThread reference to the current thread.
>>
>> A new test using jstack was also added.
>>
>> Testing:
>>   - mach5 tiers 1 - 3
>>   - the new test on Windows/Linux/OSX x86 and Solaris sparc, in 
>> regular and Xcomp mode
>>   - All jstack/stack-dump using tests I could see (linux x64):
>>     - runtime/Thread
>>     - serviceability/sa/
>>     - serviceability/tmtools/jstack/
>>     - vmTestbase/nsk/jvmti/scenarios/sampling/
>>     - vmTestbase/nsk/jvmti/MonitorWaited/ .
>>     - vmTestbase/nsk/jvmti/GetOwnedMonitorInfo/
>>
>> Thanks,
>> David
>