Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8203817

Monitor::try_lock() should not call check_prelock_state()

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • P3
    • 11
    • 11
    • hotspot
    • None
    • b16

    Backports

      Description

        In debug builds, Monitor::try_lock() calls check_prelock_state(), to check the thread state, etc. The intention is to verify that the call is made from the correct context, a context where we're allowed to block and potentially safepoint. Unlike Monitor::lock(), Monitor::try_lock() will never block, hence the call to check_prelock_state() is overly strict and we should remove it. Removing it would match the behavior of all other non-blocking functions, like Monitor::lock_without_safepoint_check(), which doesn't call check_prelock_state() either (for a good reason).

        The specific problem I've run into with this is related to JFR. Monitor::try_lock() is by JFR to allow non-blocking event generation, so that you can generate JFR events from "any" context without risk blocking/safepointing (the logic is doing something like, if try_lock() fails then put the event on a different queue and let the next owner of the lock handle it). The overly strict checks done by check_prelock_state() in try_lock() breaks this logic, which in turn means that you can't generate JFR event from "any" context as was intended.

        The patch to fix this is a one-liner, just remove the call to check_prelock_state().

        diff --git a/src/hotspot/share/runtime/mutex.cpp b/src/hotspot/share/runtime/mutex.cpp
        --- a/src/hotspot/share/runtime/mutex.cpp
        +++ b/src/hotspot/share/runtime/mutex.cpp
        @@ -971,7 +971,6 @@
         
         bool Monitor::try_lock() {
           Thread * const Self = Thread::current();
        - debug_only(check_prelock_state(Self));
           // assert(!thread->is_inside_signal_handler(), "don't lock inside signal handler");
         
           // Special case, where all Java threads are stopped.

        Attachments

          Issue Links

            Activity

              People

                pliden Per Liden (Inactive)
                pliden Per Liden (Inactive)
                Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                  Created:
                  Updated:
                  Resolved: