RFR(XS): 8202564: java/lang/management/ThreadMXBean/ThreadCounts.java fails

[Daniel D. Daugherty]

 > http://cr.openjdk.java.net/~mdoerr/8202564_thread_cnt/webrev.00/

src/hotspot/share/services/threadService.cpp
     You have moved the decrement of _exiting_daemon_threads_count

     L125:   if (daemon) {
     L126:     Atomic::dec(&_exiting_daemon_threads_count);
     L127:   }

     above this check:

     L129:   if (thread->is_hidden_from_external_view() ||
     L130:       thread->is_jvmti_agent_thread()) {

     and away from this other decrement:

     L136: 
_daemon_threads_count->set_value(_daemon_threads_count->get_value() - 1);

     That makes sense because the original increment of 
_exiting_daemon_threads_count
     happens here:

     L140 void ThreadService::current_thread_exiting(JavaThread* jt) {
     L141   assert(jt == JavaThread::current(), "Called by current thread");
     L142   Atomic::inc(&_exiting_threads_count);
     L143
     L144   oop threadObj = jt->threadObj();
     L145   if (threadObj != NULL && 
java_lang_Thread::is_daemon(threadObj)) {
     L146     Atomic::inc(&_exiting_daemon_threads_count);
     L147   }
     L148 }

     and is not guarded by the hidden thread or jvmti check. For other
     folks that are reading closely, these two checks are equivalent:

       if (daemon) {

     and:

       if (threadObj != NULL && java_lang_Thread::is_daemon(threadObj)) {

     Very nice catch!


I'm just guessing here, but I think the recent CompilerThread changes
now allow CompilerThreads to go thru the ThreadService::remove_thread()
code path which exposed this bug. So if there was a late query about
the number of daemon threads and a CompilerThread had exited, then the
count would be off. However, if the CompilerThread exited a little bit
later, then the test would pass...

Thumbs up!

Dan



On 5/8/18 12:03 PM, Vladimir Kozlov wrote:
> Hi Martin,
>
> Thank you for finding the cause and fixing it. Looks good.
>
> Changing to hotspot-dev alias since it is not compiler code. Let other 
> people look on it.
>
> Thanks,
> Vladimir
>
> On 5/8/18 6:50 AM, Doerr, Martin wrote:
>> Hi Vladimir,
>>
>> I’ve fixed the thread count issue.
>>
>> “_exiting_daemon_threads_count” just needs to get decremented at the 
>> right place (like “_exiting_threads_count”).
>>
>> Please review:
>>
>> http://cr.openjdk.java.net/~mdoerr/8202564_thread_cnt/webrev.00/
>>
>> Best regards,
>>
>> Martin
>>