RFR: 8320318: ObjectMonitor Responsible thread [v3]

[Coleen Phillimore]

On Thu, 19 Sep 2024 19:09:38 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> I think the name should describe what setting the value actually does, but if it is just a hack to do what the comment bellow says, then it sounds like a friend declaration for `SharedRuntime::monitor_exit_helper()` is what is wanted. (Or make TryLock() public.)
>>> Set check_owner to false (it's default value is true) if you want
>>> to use ObjectMonitor::try_enter() as a public way of doing TryLock().
>>> Used this way in SharedRuntime::monitor_exit_helper().
>
> Maybe `check_already_owned`? FTR I prefer this version than a friend declaration. Using it outside would also imply renaming it as try_lock to be consistent. And having to expose and check for TryLockResult is also uglier in my opinion than checking a boolean. I don't see it as a hack, we are just skipping the already owned case since we know in this case it will fail. I would actually remove that comment altogether.

I like "check_for_recusion" as a parameter name and also agree that dropping the comment, or rewriting the comment as:

    // If called from SharedRuntime::monitor_exit_helper, we know that this thread doesn't already own the lock

I agree that I don't want TryLock and its results exposed outside this file.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19454#discussion_r1767509133