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

JavaThread::check_is_terminated() implementation should rely on Thread-SMR

    XMLWordPrintable

Details

    • Enhancement
    • Resolution: Fixed
    • P4
    • 20
    • 20
    • hotspot
    • b07

    Description

      In the code review for:
          JDK-8288497 add support for JavaThread::is_oop_safe()

      this topic came up:
          https://github.com/openjdk/jdk19/pull/20#discussion_r898567223

      Here's [~dholmes]' comment:

      > Existing: I don't think I buy this argument that we need to check all the states we don't
      > want to see rather than the one we do. If the JavaThread has been freed we could
      > see anything - and we should never be executing this code in the first place.

      Here's the relevant code in JDK20:

      src/hotspot/share/runtime/javaThread.hpp:

        // thread is terminated (no longer on the threads list); we compare
        // against the three non-terminated values so that a freed JavaThread
        // will also be considered terminated.
        bool check_is_terminated(TerminatedTypes l_terminated) const {
          return l_terminated != _not_terminated && l_terminated != _thread_exiting &&
                 l_terminated != _thread_gc_barrier_detached;
        }

      The above implementation is a carry over from the pre-Thread-SMR
      days when a JavaThread could be freed while another thread had
      a reference to it. That "best guess" implementation checked for the
      two non-terminated values in the hopes that a freed JavaThread
      had not been recycled and overwritten yet. And if it was, then we
      hoped that the new use didn't put one of the two non-terminated
      values into the memory location. Of course, if the freed JavaThread
      was on a memory page that was subsequently freed, all bets were
      off and a crash was likely to result.

      Now that we have Thread-SMR, we don't have to worry about the
      memory being freed out from under us while we check the field so
      we should only have to check for "_thread_terminated". We do have
      to think about what to do if the "_vm_exited" value is seen. The
      current implementation will treat the "_vm_exited" value as terminated.

      Attachments

        Issue Links

          Activity

            People

              dcubed Daniel Daugherty
              dcubed Daniel Daugherty
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: