RFR: 8274794: Print all owned locks in hs_err file [v2]

[Coleen Phillimore]

On Sat, 16 Oct 2021 06:36:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Make mutex_array be a linked list.
>
> src/hotspot/share/runtime/mutex.cpp line 273:
> 
>> 271: // Array used to print owned locks on error.
>> 272: static Mutex* _mutex_array = NULL;
>> 273: static int _array_count = 0;
> 
> Do we need this counter? Could we not just count the mutexes while printing them?

Yes, I could change it to count while iterating.  I thought I wanted it for debugging purposes, that's why I made it static.

> src/hotspot/share/runtime/mutex.cpp line 322:
> 
>> 320:     }
>> 321:     _array_count += 1;
>> 322:   }
> 
> Proposal: maybe factor this block (and its counterpart in ~Mutex) to an own private method, "add_to_global_mutex_list" and "remove_from_global_mutex_list"? That would also make the difference between `_next` and `_next_mutex` a bit clearer to the casual reader.

Ok, I almost did this but then didn't want to change the header file to add functions or accessors.

> src/hotspot/share/runtime/mutex.cpp line 335:
> 
>> 333: #ifdef ASSERT
>> 334:   if (_allow_vm_block) {
>> 335:     st->print("%s", " allow_vm_block");
> 
> nit: shorten to `print_raw(" alloc_vm_block")` ?

ok

> src/hotspot/share/runtime/mutex.cpp line 345:
> 
>> 343: // by fatal error handler.
>> 344: void Mutex::print_owned_locks_on_error(outputStream* st) {
>> 345:   ResourceMark rm;
> 
> Why is this mark needed?

rank_name() uses resource allocation.

> src/hotspot/share/runtime/mutex.cpp line 346:
> 
>> 344: void Mutex::print_owned_locks_on_error(outputStream* st) {
>> 345:   ResourceMark rm;
>> 346:   st->print("VM Mutex/Monitor currently owned by a thread: ");
> 
> nit: use plural? 
> e.g. "All VM Mutexes/Monitors currently owned by any thread:"

ok.

> src/hotspot/share/runtime/mutex.cpp line 361:
> 
>> 359:   }
>> 360:   if (none) st->print_cr("None");
>> 361:   st->print_cr("Total Mutex count %d", _array_count);
> 
> This prints the total number of mutexes, not those owned by a thread, is that intentional?

yeah I wanted to know how many we have total in the array.

> src/hotspot/share/utilities/vmError.cpp line 1920:
> 
>> 1918:       MutexLocker ml(ErrorTest_lock, Mutex::_no_safepoint_check_flag);
>> 1919:       assert(how == 0, "test assert with lock");
>> 1920:       break;
> 
> I like this test.

Thank you!

-------------

PR: https://git.openjdk.java.net/jdk/pull/5958