RFR: 8263896: Remove not_suspended parameter from ObjectMonitor::exit() [v3]

[Patricio Chilano Mateo]

On Fri, 26 Mar 2021 19:17:43 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> Hi Dan,
>> 
>>> Instead of gating this piece of code on a flag that's passed in from
>>> the code in enter() that calls exit() under the special case, you're
>>> switching to the `current_pending_monitor` field which is set and
>>> cleared by the code in enter(). It's only set when there is a contended
>>> enter and only cleared when we've successfully entered after a
>>> contended enter. Entering while there's a pending suspend request is
>>> not counted as a successful enter which is why the original code has
>>> that flag to exit().
>>> 
>>> I think this change makes the linkage between the semantics of
>>> enter() and exit() more brittle and I'm not fond of this. However,
>>> I agree that this change makes the linkage between enter() and
>>> exit() an implementation detail instead of exposed by the API. Of
>>> course, only someone that is intimately familiar with the
>>> implementation is going to even grok the reasons for that 3-line
>>> block of code.
>>> 
>>> Okay. Originally I was going to object, but I've decided that making
>>> this an implementation detail is better.
>> Thanks for the review. David would rather keep the use of the boolean here and just change it to have a default value of true, since that avoids using side state to decide whether we should set or not the previous owner. I prefer this approach of removing it altogether from the definition because I think this boolean is just a consequence of how we implement enter() rather than something the user of this class really cares about specifying when calling exit(). But in any case I don't mind using a parameter with default value since that also avoids having the callers of exit() worry about it. Do you also would rather keep not_suspended and set a default value of 'true' instead? If so, can I use this same PR to do that (because it says 'Remove' on the title).
>
> @pchilano - I'm good with however you decide to solve this issue.
> Normally, I hate default parameters, but this is a case where I can
> see that having a default value of `true` makes sense.

Ok, thanks Dan. Please review the update. I still kept the assert() on current_pending_monitor. Should I modify the title of the PR?

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

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