# HG changeset patch
# User dlong
# Date 1539813672 25200
#      Wed Oct 17 15:01:12 2018 -0700
# Node ID fafde0f59e3a63847f2135d87c390aebce45c6ce
# Parent  367b2cd49ec5efad2f89346d3c0d1ddcc115d70c
[mq]: 8021335

diff --git a/src/hotspot/share/runtime/thread.cpp b/src/hotspot/share/runtime/thread.cpp
--- a/src/hotspot/share/runtime/thread.cpp
+++ b/src/hotspot/share/runtime/thread.cpp
@@ -1819,6 +1819,9 @@
   thread->clear_pending_exception();
 }
 
+static bool is_daemon(oop threadObj) {
+  return (threadObj != NULL && java_lang_Thread::is_daemon(threadObj));
+}
 
 // For any new cleanup additions, please check to see if they need to be applied to
 // cleanup_failed_attach_current_thread as well.
@@ -1910,7 +1913,7 @@
         MutexLockerEx ml(SR_lock(), Mutex::_no_safepoint_check_flag);
         if (!is_external_suspend()) {
           set_terminated(_thread_exiting);
-          ThreadService::current_thread_exiting(this);
+          ThreadService::current_thread_exiting(this, is_daemon(threadObj()));
           break;
         }
         // Implied else:
@@ -1930,6 +1933,7 @@
     }
     // no more external suspends are allowed at this point
   } else {
+    assert(!is_terminated() && !is_exiting(), "must not be exiting");
     // before_exit() has already posted JVMTI THREAD_END events
   }
 
@@ -4332,7 +4336,7 @@
 
 void Threads::add(JavaThread* p, bool force_daemon) {
   // The threads lock must be owned at this point
-  assert_locked_or_safepoint(Threads_lock);
+  assert(Threads_lock->owned_by_self(), "must have threads lock");
 
   BarrierSet::barrier_set()->on_thread_attach(p);
 
@@ -4348,7 +4352,7 @@
   bool daemon = true;
   // Bootstrapping problem: threadObj can be null for initial
   // JavaThread (or for threads attached via JNI)
-  if ((!force_daemon) && (threadObj == NULL || !java_lang_Thread::is_daemon(threadObj))) {
+  if ((!force_daemon) && !is_daemon((threadObj))) {
     _number_of_non_daemon_threads++;
     daemon = false;
   }
@@ -4393,7 +4397,7 @@
     _number_of_threads--;
     oop threadObj = p->threadObj();
     bool daemon = true;
-    if (threadObj == NULL || !java_lang_Thread::is_daemon(threadObj)) {
+    if (!is_daemon(threadObj)) {
       _number_of_non_daemon_threads--;
       daemon = false;
 
diff --git a/src/hotspot/share/services/threadService.cpp b/src/hotspot/share/services/threadService.cpp
--- a/src/hotspot/share/services/threadService.cpp
+++ b/src/hotspot/share/services/threadService.cpp
@@ -57,8 +57,8 @@
 PerfVariable* ThreadService::_live_threads_count = NULL;
 PerfVariable* ThreadService::_peak_threads_count = NULL;
 PerfVariable* ThreadService::_daemon_threads_count = NULL;
-volatile int ThreadService::_exiting_threads_count = 0;
-volatile int ThreadService::_exiting_daemon_threads_count = 0;
+volatile int ThreadService::_atomic_threads_count = 0;
+volatile int ThreadService::_atomic_daemon_threads_count = 0;
 
 ThreadDumpResult* ThreadService::_threaddump_list = NULL;
 
@@ -101,50 +101,104 @@
   _peak_threads_count->set_value(get_live_thread_count());
 }
 
+static bool is_hidden_thread(JavaThread *thread) {
+  // hide VM internal or JVMTI agent threads
+  return thread->is_hidden_from_external_view() || thread->is_jvmti_agent_thread();
+}
+
 void ThreadService::add_thread(JavaThread* thread, bool daemon) {
-  // Do not count VM internal or JVMTI agent threads
-  if (thread->is_hidden_from_external_view() ||
-      thread->is_jvmti_agent_thread()) {
+  assert(Threads_lock->owned_by_self(), "must have threads lock");
+
+  // Do not count hidden threads
+  if (is_hidden_thread(thread)) {
     return;
   }
 
   _total_threads_count->inc();
   _live_threads_count->inc();
+  Atomic::inc(&_atomic_threads_count);
+  int count = _atomic_threads_count;
 
-  if (_live_threads_count->get_value() > _peak_threads_count->get_value()) {
-    _peak_threads_count->set_value(_live_threads_count->get_value());
+  if (count > _peak_threads_count->get_value()) {
+    _peak_threads_count->set_value(count);
   }
 
   if (daemon) {
     _daemon_threads_count->inc();
+    Atomic::inc(&_atomic_daemon_threads_count);
+  }
+}
+
+void ThreadService::decrement_thread_counts(JavaThread* jt, bool daemon) {
+  Atomic::dec(&_atomic_threads_count);
+
+  if (daemon) {
+    Atomic::dec(&_atomic_daemon_threads_count);
   }
 }
 
 void ThreadService::remove_thread(JavaThread* thread, bool daemon) {
-  Atomic::dec(&_exiting_threads_count);
-  if (daemon) {
-    Atomic::dec(&_exiting_daemon_threads_count);
-  }
+  assert(Threads_lock->owned_by_self(), "must have threads lock");
 
-  if (thread->is_hidden_from_external_view() ||
-      thread->is_jvmti_agent_thread()) {
+  // Do not count hidden threads
+  if (is_hidden_thread(thread)) {
     return;
   }
 
-  _live_threads_count->set_value(_live_threads_count->get_value() - 1);
+  assert(!thread->is_terminated(), "must not be terminated");
+  if (!thread->is_exiting()) {
+    // JavaThread::exit() skipped calling current_thread_exiting()
+    decrement_thread_counts(thread, daemon);
+  }
+
+  int daemon_count = _atomic_daemon_threads_count;
+  int count = _atomic_threads_count;
+
+  // Counts are incremented at the same time, but atomic counts are
+  // decremented earlier than perf counts.
+  assert(_live_threads_count->get_value() > count,
+    "thread count mismatch %d : %d",
+    (int)_live_threads_count->get_value(), count);
+
+  _live_threads_count->dec(1);
   if (daemon) {
-    _daemon_threads_count->set_value(_daemon_threads_count->get_value() - 1);
+    assert(_daemon_threads_count->get_value() > daemon_count,
+      "thread count mismatch %d : %d",
+      (int)_daemon_threads_count->get_value(), daemon_count);
+
+    _daemon_threads_count->dec(1);
   }
+
+  // Counts are incremented at the same time, but atomic counts are
+  // decremented earlier than perf counts.
+  assert(_daemon_threads_count->get_value() >= daemon_count,
+    "thread count mismatch %d : %d",
+    (int)_daemon_threads_count->get_value(), daemon_count);
+  assert(_live_threads_count->get_value() >= count,
+    "thread count mismatch %d : %d",
+    (int)_live_threads_count->get_value(), count);
+  assert(_live_threads_count->get_value() > 0 ||
+    (_live_threads_count->get_value() == 0 && count == 0 &&
+    _daemon_threads_count->get_value() == 0 && daemon_count == 0),
+    "thread counts should reach 0 at the same time, live %d,%d daemon %d,%d",
+    (int)_live_threads_count->get_value(), count,
+    (int)_daemon_threads_count->get_value(), daemon_count);
+  assert(_daemon_threads_count->get_value() > 0 ||
+    (_daemon_threads_count->get_value() == 0 && daemon_count == 0),
+    "thread counts should reach 0 at the same time, daemon %d,%d",
+    (int)_daemon_threads_count->get_value(), daemon_count);
 }
 
-void ThreadService::current_thread_exiting(JavaThread* jt) {
+void ThreadService::current_thread_exiting(JavaThread* jt, bool daemon) {
+  // Do not count hidden threads
+  if (is_hidden_thread(jt)) {
+    return;
+  }
+
   assert(jt == JavaThread::current(), "Called by current thread");
-  Atomic::inc(&_exiting_threads_count);
+  assert(!jt->is_terminated() && jt->is_exiting(), "must be exiting");
 
-  oop threadObj = jt->threadObj();
-  if (threadObj != NULL && java_lang_Thread::is_daemon(threadObj)) {
-    Atomic::inc(&_exiting_daemon_threads_count);
-  }
+  decrement_thread_counts(jt, daemon);
 }
 
 // FIXME: JVMTI should call this function
diff --git a/src/hotspot/share/services/threadService.hpp b/src/hotspot/share/services/threadService.hpp
--- a/src/hotspot/share/services/threadService.hpp
+++ b/src/hotspot/share/services/threadService.hpp
@@ -58,10 +58,12 @@
   static PerfVariable* _peak_threads_count;
   static PerfVariable* _daemon_threads_count;
 
-  // These 2 counters are atomically incremented once the thread is exiting.
-  // They will be atomically decremented when ThreadService::remove_thread is called.
-  static volatile int  _exiting_threads_count;
-  static volatile int  _exiting_daemon_threads_count;
+  // These 2 counters are like the above thread counts, but are
+  // atomically decremented in ThreadService::current_thread_exiting instead of
+  // ThreadService::remove_thread, so that the thread count is updated before
+  // Thread.join() returns.
+  static volatile int  _atomic_threads_count;
+  static volatile int  _atomic_daemon_threads_count;
 
   static bool          _thread_monitoring_contention_enabled;
   static bool          _thread_cpu_time_enabled;
@@ -72,11 +74,13 @@
   // requested by multiple threads concurrently.
   static ThreadDumpResult* _threaddump_list;
 
+  static void decrement_thread_counts(JavaThread* jt, bool daemon);
+
 public:
   static void init();
   static void add_thread(JavaThread* thread, bool daemon);
   static void remove_thread(JavaThread* thread, bool daemon);
-  static void current_thread_exiting(JavaThread* jt);
+  static void current_thread_exiting(JavaThread* jt, bool daemon);
 
   static bool set_thread_monitoring_contention(bool flag);
   static bool is_thread_monitoring_contention() { return _thread_monitoring_contention_enabled; }
@@ -89,11 +93,8 @@
 
   static jlong get_total_thread_count()       { return _total_threads_count->get_value(); }
   static jlong get_peak_thread_count()        { return _peak_threads_count->get_value(); }
-  static jlong get_live_thread_count()        { return _live_threads_count->get_value() - _exiting_threads_count; }
-  static jlong get_daemon_thread_count()      { return _daemon_threads_count->get_value() - _exiting_daemon_threads_count; }
-
-  static int   exiting_threads_count()        { return _exiting_threads_count; }
-  static int   exiting_daemon_threads_count() { return _exiting_daemon_threads_count; }
+  static jlong get_live_thread_count()        { return _atomic_threads_count; }
+  static jlong get_daemon_thread_count()      { return _atomic_daemon_threads_count; }
 
   // Support for thread dump
   static void   add_thread_dump(ThreadDumpResult* dump);
diff --git a/test/jdk/java/lang/management/ThreadMXBean/ResetPeakThreadCount.java b/test/jdk/java/lang/management/ThreadMXBean/ResetPeakThreadCount.java
--- a/test/jdk/java/lang/management/ThreadMXBean/ResetPeakThreadCount.java
+++ b/test/jdk/java/lang/management/ThreadMXBean/ResetPeakThreadCount.java
@@ -158,10 +158,6 @@
                 " Expected to be == previous peak = " + peak1 + " + " +
                 delta);
         }
-        // wait until the current thread count gets incremented
-        while (mbean.getThreadCount() < (current + count)) {
-            Thread.sleep(100);
-        }
         current = mbean.getThreadCount();
         System.out.println("   Live thread count before returns " + current);
         return current;
@@ -195,12 +191,6 @@
             allThreads[i].join();
         }
 
-        // there is a race in the counter update logic somewhere causing
-        // the thread counters go ff
-        // we need to give the terminated threads some extra time to really die
-        // JDK-8021335
-        Thread.sleep(500);
-
         long current = mbean.getThreadCount();
         System.out.println("   Live thread count before returns " + current);
         return current;
# HG changeset patch
# User dlong
# Date 1539814482 25200
#      Wed Oct 17 15:14:42 2018 -0700
# Node ID 3ebbc4e06d6a6c85aeaf1779445c8dfe79acdd79
# Parent  fafde0f59e3a63847f2135d87c390aebce45c6ce
[mq]: 8021335.diff

diff --git a/src/hotspot/share/services/threadService.cpp b/src/hotspot/share/services/threadService.cpp
--- a/src/hotspot/share/services/threadService.cpp
+++ b/src/hotspot/share/services/threadService.cpp
@@ -115,25 +115,27 @@
   }
 
   _total_threads_count->inc();
-  _live_threads_count->inc();
   Atomic::inc(&_atomic_threads_count);
   int count = _atomic_threads_count;
+  _live_threads_count->set_value(count);
 
   if (count > _peak_threads_count->get_value()) {
     _peak_threads_count->set_value(count);
   }
 
   if (daemon) {
-    _daemon_threads_count->inc();
     Atomic::inc(&_atomic_daemon_threads_count);
+    _daemon_threads_count->set_value(_atomic_daemon_threads_count);
   }
 }
 
 void ThreadService::decrement_thread_counts(JavaThread* jt, bool daemon) {
   Atomic::dec(&_atomic_threads_count);
+  _live_threads_count->set_value(_atomic_threads_count);
 
   if (daemon) {
     Atomic::dec(&_atomic_daemon_threads_count);
+    _daemon_threads_count->set_value(_atomic_daemon_threads_count);
   }
 }
 
@@ -151,42 +153,6 @@
     decrement_thread_counts(thread, daemon);
   }
 
-  int daemon_count = _atomic_daemon_threads_count;
-  int count = _atomic_threads_count;
-
-  // Counts are incremented at the same time, but atomic counts are
-  // decremented earlier than perf counts.
-  assert(_live_threads_count->get_value() > count,
-    "thread count mismatch %d : %d",
-    (int)_live_threads_count->get_value(), count);
-
-  _live_threads_count->dec(1);
-  if (daemon) {
-    assert(_daemon_threads_count->get_value() > daemon_count,
-      "thread count mismatch %d : %d",
-      (int)_daemon_threads_count->get_value(), daemon_count);
-
-    _daemon_threads_count->dec(1);
-  }
-
-  // Counts are incremented at the same time, but atomic counts are
-  // decremented earlier than perf counts.
-  assert(_daemon_threads_count->get_value() >= daemon_count,
-    "thread count mismatch %d : %d",
-    (int)_daemon_threads_count->get_value(), daemon_count);
-  assert(_live_threads_count->get_value() >= count,
-    "thread count mismatch %d : %d",
-    (int)_live_threads_count->get_value(), count);
-  assert(_live_threads_count->get_value() > 0 ||
-    (_live_threads_count->get_value() == 0 && count == 0 &&
-    _daemon_threads_count->get_value() == 0 && daemon_count == 0),
-    "thread counts should reach 0 at the same time, live %d,%d daemon %d,%d",
-    (int)_live_threads_count->get_value(), count,
-    (int)_daemon_threads_count->get_value(), daemon_count);
-  assert(_daemon_threads_count->get_value() > 0 ||
-    (_daemon_threads_count->get_value() == 0 && daemon_count == 0),
-    "thread counts should reach 0 at the same time, daemon %d,%d",
-    (int)_daemon_threads_count->get_value(), daemon_count);
 }
 
 void ThreadService::current_thread_exiting(JavaThread* jt, bool daemon) {