RFR(S) 8130448: thread dump improvements, comment, additions, new diagnostics inspired by 8077392

[Daniel D. Daugherty]

Need a second reviewer for this bug fix.

One minor tweak has been done relative to the webrev below:

$ diff -c src/share/vm/runtime/synchronizer.cpp{.cr1,}
*** src/share/vm/runtime/synchronizer.cpp.cr1   Wed Jul  8 07:15:07 2015
--- src/share/vm/runtime/synchronizer.cpp       Thu Jul  9 07:07:28 2015
***************
*** 1651,1661 ****
           Handle obj((oop) mid->object());
           tty->print("INFO: unexpected locked object:");
           javaVFrame::print_locked_object_class_name(tty, obj, "locked");
         }
-       guarantee(ObjectMonitor::Knob_VerifyMatch == 0,
-                 err_msg("exiting JavaThread=" INTPTR_FORMAT
-                         " unexpectedly owns ObjectMonitor=" INTPTR_FORMAT,
-                         THREAD, mid));
         (void)mid->complete_exit(CHECK);
       }
     }
--- 1651,1660 ----
           Handle obj((oop) mid->object());
           tty->print("INFO: unexpected locked object:");
           javaVFrame::print_locked_object_class_name(tty, obj, "locked");
+         fatal(err_msg("exiting JavaThread=" INTPTR_FORMAT
+                       " unexpectedly owns ObjectMonitor=" INTPTR_FORMAT,
+                       THREAD, mid));
         }
         (void)mid->complete_exit(CHECK);
       }
     }

Dan


On 7/8/15 3:33 PM, Daniel D. Daugherty wrote:
> Greetings,
>
> I've updated this patch based on David H's comments along with
> some additional cleanup.
>
> This work is being tracked by the following bug ID:
>
>     JDK-8130448 8077392 inspired thread dump improvements, comment
>                 additions, new diagnostics
>     https://bugs.openjdk.java.net/browse/JDK-8130448
>
> Here is the webrev URL:
>
> http://cr.openjdk.java.net/~dcubed/8130448-webrev/1-jdk9-hs-rt/
>
> The easiest way to review the delta is to download the two patch
> files and compare them in something like jfilemerge:
>
> http://cr.openjdk.java.net/~dcubed/8130448-webrev/0-jdk9-hs-rt/hotspot.patch 
>
> http://cr.openjdk.java.net/~dcubed/8130448-webrev/1-jdk9-hs-rt/hotspot.patch 
>
>
> Testing:
>
> - Aurora Adhoc RT/SVC nightly batches (in process)
> - JPRT test jobs
>
> Thanks, in advance, for any comments, questions or suggestions.
>
> Gory details about the changes are below...
>
> Dan
>
> Changes between CR0 and CR1 are:
>
> src/cpu/x86/vm/macroAssembler_x86.cpp
>
>   - fix comment typos, clarify comment wording
>
> src/share/vm/oops/markOop.cpp
>
>   - clarify output messages
>   - get rid of confusing local variable
>
> src/share/vm/runtime/globals.hpp
>
>   - drop VerboseStackTrace, GuaranteeOnMonitorMismatch
>     and JavaThreadExitReleasesMonitors options
>   - there are no longer changes to this file
>
> src/share/vm/runtime/objectMonitor.cpp
>
>   - add ObjectMonitor::Knob_ExitRelease suboption to replace
>     JavaThreadExitReleasesMonitors
>   - add ObjectMonitor::Knob_VerifyMatch suboption to replace
>     GuaranteeOnMonitorMismatch
>   - cleanup messy logging:
>     - switch from '::printf()' -> 'tty->print_cr()'
>     - switch from '::fflush()' -> 'tty->flush()'
>
> src/share/vm/runtime/objectMonitor.hpp
>
>   - add ObjectMonitor::Knob_ExitRelease suboption
>   - add ObjectMonitor::Knob_VerifyMatch suboption
>   - cleanup messy logging
>
> src/share/vm/runtime/synchronizer.cpp
>
>   - cleanup messy logging
>   - switch from GuaranteeOnMonitorMismatch to
>     ObjectMonitor::Knob_VerifyMatch
>   - add diagnostic information about the locked object
>     when ReleaseJavaMonitorsClosure::do_monitor() finds
>     a problem
>
> src/share/vm/runtime/thread.cpp
>
>   - clarify comments
>   - switch from JavaThreadExitReleasesMonitors to
>     ObjectMonitor::Knob_ExitRelease
>
> src/share/vm/runtime/vframe.cpp
>
>   - print_locked_object_class_name() is now public
>   - clarify comments
>   - clarify output messages
>   - switch from VerboseStackTrace to the already existing
>     ObjectMonitor::Knob_Verbose
>
> src/share/vm/runtime/vframe.hpp
>
>   - print_locked_object_class_name() is now public
>
>
> On 7/3/15 6:14 PM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> The hunt for the following bug:
>>
>>     JDK-8077392 Stream fork/join tasks occasionally fail to complete
>>
>> and David C's work on the following bug:
>>
>>     JDK-8069412 Locks need better debug-printing support
>>
>> have inspired additional thread dump improvements, comment additions
>> to some Java monitor code and some new diagnostic options.
>>
>> This work is being tracked by the following bug ID:
>>
>>     JDK-8130448 8077392 inspired thread dump improvements, comment
>>                 additions, new diagnostics
>>     https://bugs.openjdk.java.net/browse/JDK-8130448
>>
>> Here is the webrev URL:
>>
>> http://cr.openjdk.java.net/~dcubed/8130448-webrev/0-jdk9-hs-rt/
>>
>> Testing:
>>
>> - RBT vm.quick batches (in process)
>> - JPRT test jobs
>>
>> Thanks, in advance, for any comments, questions or suggestions.
>>
>> Gory details about the changes are below...
>>
>> Dan
>>
>> 8130448 summary of changes:
>>
>> src/cpu/x86/vm/macroAssembler_x86.cpp
>>   - comment additions for the assembly code
>>
>> src/share/vm/oops/markOop.cpp
>>   - has_monitor() has to be checked before is_locked() since
>>     the has_monitor() bits are a superset of the is_locked() bits
>>   - code style fixes
>>
>> src/share/vm/runtime/globals.hpp
>>   - add VerboseStackTrace diagnostic option
>>   - add GuaranteeOnMonitorMismatch diagnostic option
>>   - add JavaThreadExitReleasesMonitors product option;
>>     this is the Java equivalent of JNIDetachReleasesMonitors
>>
>> src/share/vm/runtime/objectMonitor.cpp
>>   - delete unused ObjectMonitor::try_enter()
>>   - fix assert wording
>>
>> src/share/vm/runtime/objectMonitor.hpp
>>   - delete unused ObjectMonitor::try_enter()
>>
>> src/share/vm/runtime/synchronizer.cpp
>>   - add GuaranteeOnMonitorMismatch support with some
>>     diagnostic output
>>
>> src/share/vm/runtime/thread.cpp
>>   - add JavaThreadExitReleasesMonitors support
>>
>> src/share/vm/runtime/vframe.cpp
>>   - clarify existing comments
>>   - add comments to clarify what "waiting on" means
>>   - add line to show when we are "waiting on", but
>>     there are no locals available (previously we were
>>     "waiting on" silently)
>>   - add support for "waiting to relock" which can happen
>>     for any of the monitors and not just the top monitor
>>   - switch mark->print_on() verbose output to new
>>     VerboseStackTrace switch; thread dumps are not enabled
>>     with a specific switch so the '-XX:+Verbose' model of
>>     being a modifier for another option does not fit
>>
>>
>
>
>