RFR (S): 8218483: Crash in "assert(_daemon_threads_count->get_value() > daemon_count) failed: thread count mismatch 5 : 5"

[David Holmes]

Hi Dan,

Thanks for taking a look at this.

On 3/04/2019 5:17 am, Daniel D. Daugherty wrote:
> On 4/1/19 6:30 PM, David Holmes wrote:
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8218483
>> webrev: http://cr.openjdk.java.net/~dholmes/8218483/webrev/
> 
> src/hotspot/share/prims/jni.cpp
>      No comments.
> 
> src/hotspot/share/runtime/thread.hpp
>      No comments.
> 
> src/hotspot/share/runtime/thread.cpp
>      No comments.
> 
> test/hotspot/gtest/threadHelper.inline.hpp
>      No comments.
> 
> Thumbs up!

Thanks.

> I sanity checked uses of java_lang_Thread::set_daemon() and didn't see any
> issues there. I also checked uses of java_lang_Thread::is_daemon() and the
> most I found was a possible issue with thread dump output racing with a
> Thread.setDaemon(false) call after the thread has died. However, that
> would require that the ThreadsList captured for the thread dump contain
> the JavaThread that's about to exit, the JavaThread exiting, another
> thread calling Thread.setDaemon(false) before the thread dump gets to
> querying the JavaThread that is trying to exit... Pretty rare if possible
> at all and most that could go wrong with the output is a missing "daemon"
> marker in the thread dump...

Thanks for looking more deeply at that.

There's another race between setDaemon(true) and start() that could lead 
to the same problem resurfacing, due to a lack of synchronization in 
setDaemon. I decided to let JDK-8221657 fix that, as there's no test 
that exposes the issue.

Thanks,
David

> Dan
> 
> 
>>
>> A bug in Thread.setDaemon (JDK-8221657) means that the daemon state of 
>> a thread can change after the thread is !isAlive() at the Java level. 
>> If this happens before the VM call to ThreadService::remove_thread 
>> then we have a situation where we incremented the thread counters when 
>> the thread was not a daemon, and we decrement the thread counters when 
>> the thread is a daemon - and so the counters are out of sync and the 
>> assertion fires.
>>
>> The simple fix is to capture the daemon state of the thread while it 
>> is still alive and to pass that through to Threads::remove and thus 
>> ThreadService::remove_thread.
>>
>> Testing:
>>   - manual test with modified VM (to delay Threads::remove call) as 
>> per the bug report
>>   - mach 5 tiers 1-3
>>
>> Thanks,
>> David
>>
>