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

[Doerr, Martin]

Hi Vladimir and Daniel,

thank you for reviewing and pushing.

Best regards,
Martin


-----Original Message-----
From: Daniel D. Daugherty [mailto:daniel.daugherty at oracle.com] 
Sent: Dienstag, 8. Mai 2018 18:24
To: Vladimir Kozlov <vladimir.kozlov at oracle.com>; Doerr, Martin <martin.doerr at sap.com>; hotspot-dev Source Developers <hotspot-dev at openjdk.java.net>
Subject: Re: RFR(XS): 8202564: java/lang/management/ThreadMXBean/ThreadCounts.java fails

 > 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
>>