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

[David Holmes]

Hi Thomas,

Thanks for taking a look at this.

On 3/04/2019 5:41 am, Thomas Stüfe wrote:
> Hi David,
> 
> first thanks for the good analysis!
> 
> Is this not a problem with the usage of setDaemon():
> 
> https://docs.oracle.com/javase/7/docs/api/java/lang/Thread.html#setDaemon(boolean)
> 
> "This method must be invoked before the thread is started."

Not the usage as such, but there is a problem with setDaemon - as per:

https://bugs.openjdk.java.net/browse/JDK-8221657

The test that causes the crash in the VM deliberately tests a case where 
it expects to get the IllegalThreadStateException.

> I think the real solution would be for setDaemon to distinguish between 
> not-yet-started, running and finished. It should not use isAlive(). It 
> should throw an exception if it has been started, regardless of whether 
> it finished already or not.

Yes that fix is needed at the Java level. The use of isAlive() pre-dates 
the existence of Thread.State.

But a change at the Java level may be some time coming given this is a 
day one bug in the spec and implementation of Thread.setDaemon, so I 
wanted to address this quickly in the VM as we are seeing these crashes 
in testing.

Thanks,
David

> Not sure. Its late, I may not be thinking straight.
> 
> Cheers, Thomas
> 
> 
> 
> On Tue, Apr 2, 2019 at 12:33 AM David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8218483
>     webrev: http://cr.openjdk.java.net/~dholmes/8218483/webrev/
> 
>     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
>