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

[David Holmes]

Hi Robbin,

On 14/01/2019 11:08 pm, David Holmes wrote:
> Hi Robbin,
> 
> On 14/01/2019 6:54 pm, Robbin Ehn wrote:
>> Hi David, looks good.
> 
> Thanks for the review.
> 
>> In the test I would prefer e.g. semaphore acquire/release instead of 
>> boolean ready with sleep. (in this case it doesn't seem to make much 
>> of a difference)
>> So do as you wish, I do not need to see a new webrev if you choose to 
>> change.
> 
> Okay. Path of least resistance ...

Actually there's a reason not to use something like Semaphore and that 
is because we want the "ready" signal to happen as close as possible to 
the static initialization attempt. If I used a semaphore there's a much 
greater chance we will still be inside Semaphore code when the jstack is 
executed.

This realization also made me realise that using "new Target()" to 
trigger static initialization is not the best thing to do because it 
will perform the allocation first, and then call the constructor that 
will result in the static init. So I've changed it to access a static 
field instead.

Updated webrev:

http://cr.openjdk.java.net/~dholmes/8213397/webrev.v2

This also addresses Coleen's suggestion to remove the assert in the 
vframe code.

Thanks,
David
-----


> Thanks,
> David
> 
>> Thanks, Robbin
>>
>> On 1/14/19 7: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