RFR(S): 8229212 clear up CHECK_OWNER confusion in objectMonitor.cpp

[coleen.phillimore at oracle.com]

This change looks good.  I was wondering why check_owner had to return 
boolean until the last source file change.

http://cr.openjdk.java.net/~dcubed/8229212-webrev/1_for_jdk14.full/test/hotspot/jtreg/runtime/Monitor/NonOwnerOps.java.html

This took me way too long to figure out why there's a "before" and 
"after" condition and what the point was.

Why not just have:

40         try {
   41             obj.wait();
> 53             System.err.println("ERROR: wait() by non-owner thread did not " +
> 54                                "throw IllegalMonitorStateException.");
> 55             error_count++;

  43         } catch (InterruptedException ie) {
   44             System.err.println("ERROR: wait() by non-owner thread threw " +
   45                                "InterruptedException which is not expected.");
   46             error_count++;
   47         } catch (IllegalMonitorStateException imse) {
   48             System.out.println("wait() by non-owner thread threw the " +
   49                                "expected IllegalMonitorStateException:");
   50             System.out.println("    " + imse);
   51         }



On 8/13/19 9:49 AM, Daniel D. Daugherty wrote:
> I still need a second review... any takers?
>
> Dan
>
>
> On 8/12/19 8:57 AM, Daniel D. Daugherty wrote:
>> David, thanks for the re-review!
>>
>> I need a second review... any takers?
>>
>> Dan
>>
>>
>>
>> On 8/12/19 12:00 AM, David Holmes wrote:
>>> Looks good to me!
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>> On 10/08/2019 5:30 am, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> Updated the fix based on David H's review.
>>>>
>>>> Incremental webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8229212-webrev/1_for_jdk14.inc/
>>>>
>>>> Full webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8229212-webrev/1_for_jdk14.full/
>>>>
>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>
>>>> Dan
>>>>
>>>>
>>>> On 8/8/19 10:25 AM, Daniel D. Daugherty wrote:
>>>>> Greetings,
>>>>>
>>>>> Discussions with David Holmes about ObjectMonitors revealed a bit of
>>>>> a mess with the CHECK_OWNER macro and the ObjectMonitor::check() and
>>>>> ObjectMonitor::check_slow() functions. I have a small fix to clean up
>>>>> this bit of messy code.
>>>>>
>>>>> The bug URL:
>>>>>
>>>>>     JDK-8229212 clear up CHECK_OWNER confusion in objectMonitor.cpp
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8229212
>>>>>
>>>>> The webrev URL:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/8229212-webrev/0_for_jdk14/
>>>>>
>>>>> Summary of the fix:
>>>>>
>>>>>     Merge the CHECK_OWNER macro and the ObjectMonitor::check() and
>>>>>     ObjectMonitor::check_slow() functions into a new function:
>>>>>
>>>>>         ObjectMonitor::check_owner_and_throw_IMSE_if_not()
>>>>>
>>>>>     Update the CHECK_OWNER and check() call sites to use
>>>>>     check_owner_and_throw_IMSE_if_not(). Update the comments
>>>>>     to make the intended semantics clear.
>>>>>
>>>>> Also included a new test for when Object.wait(), Object.notify()
>>>>> and Object.notifyAll() are called a thread that doesn't own the
>>>>> Java monitor.
>>>>>
>>>>> Test with Mach5 Tier[1-3] on the usual Oracle platforms. Verified
>>>>> that the new test passes on all Oracle platforms. It's a Tier1
>>>>> test but its duration is < 1s on every platform.
>>>>>
>>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>>
>>>>> Dan
>>>>>
>>>>>
>>>>
>>
>>
>