Bug ID: JDK-6566340 Restore use of stillborn flag to signify a thread that was stopped before it started

JDK 7	Other
7Fixed	hs21Fixed

PUBLIC COMMENTS Just a follow up note as I realized some existing code in this area is incorrect/misleading. In thread_main_inner we have: // Execute thread entry point unless this thread has a pending exception // or has been stopped before starting. // Note: Due to JVM_StopThread we can have pending exceptions already! if (!this->has_pending_exception() && !java_lang_Thread::is_stillborn(this->threadObj())) { HandleMark hm(this); this->entry_point()(this, this); } but the comment about JVM_StopThread is incorrect as it sets a pending_async_exception _not_ a pending_exception. So has_pending_exception should always be false above; further we could/should check for pending_async_exception in case this thread was stopped just after it started. As it is detection of that fact will be deferred until the thread reaches a point where it checks for async exceptions. The old code doesn't have this issue because the old send_thread_stop logic always set the stillborn flag, which would be seen by the above logic.
01-02-2011
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot/hotspot/rev/ccfcb502af3f
27-01-2011
EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-rt/hotspot/rev/ccfcb502af3f
25-01-2011
PUBLIC COMMENTS There is one case where resurrection must still be handled in JVM_StartThread. When a thread attaches via JNI there is a short window where the Thread object exists, has a non-zero eetop (ie isAlive) but threadStatus is still zero. In that case an attempt to start() would not be caught at the Java level so we have to catch it in the VM.
21-01-2011
PUBLIC COMMENTS The HSX situation doe snot complicate this because all the requisite changes in logic occurred at Java 5. I also now realize that this curious logic in send_thread_stop: 1999 // This is a change from JDK 1.1, but JDK 1.2 will also do it: 2000 if (java_throwable->is_a(SystemDictionary::ThreadDeath_klass())) { 2001 java_lang_Thread::set_stillborn(threadObj()); 2002 } is the code responsible for the bug where a thread that had had stop() called on it reported that it was not alive, even though the thread can catch the ThreadDeath and continue on its merry way. This stems back to the ancient days when it was a combination of the stillborn flag and the presence of a JavaThread (the ee_top field in the java.lang.Thread) that determined the isAlive status. When that was modified to be based only on the JavaThread status the above chunk of code could have (and should have) been deleted.
18-01-2011
EVALUATION See suggested fix.
18-01-2011
PUBLIC COMMENTS Note that now that HS is used for Java 6 and 7 trying to fix this is somewhat more complicated.
09-07-2010
PUBLIC COMMENTS [moved from private comments now this is all open-source] The problems with the handling of Thread.stop before performing a Thread.start can be traced back to JDK 1.1. In 1.1 the stillborn flag was correctly used to detect that a thread had been stopped before it was started. The stop code in the VM unconditionally set stillborn and the entry routine of the new thread immediately called thread_exit if stillborn was set. This was correct behaviour. However the 1.1 code also stated that stillborn was always set by stop so that it could also act as a means to disallow "resurrection" ie starting a thread twice. But in fact the start routine used the PrivateInfo pointer to determine if this was a resurrection attempt, not the stillborn flag. What happened next is lost in history but somewhere along the way hotspot changed a few things. The stillborn flag was set by the VM_ThreadStop safepoint operation, but that only occurred if the target thread had already been started - hence stillborn was no longer usable as a check for immediate termination in the thread startup routine even though the startup routine still checked for it!. Further it appears that at some stage the value of the stillborn flag may have really been used to try and detect resurrection. And along the way there were bugs concerning what isAlive reported and when. Jump forward a few years to Java 5. Now resurrection is handled at the Java level in Thread.start() using the new threadState value. (Though a simple boolean flag could always have been used!). Jump to Java 6 and the long open bug 4519200 (presumably opened against 1.2 after stop() was deprecated) was finally addressed. However it was done in a way that I cannot agree with either in intent or implementation. The notion of "terminates immediately" has always been very clear (though I would go further and argue that stop() before start() could have been handled by start() being a no-op for a stopped thread, rather than actually creating the native thread, starting it and then let it simply terminate). But what 4519200 did was to implement stop-before-start by not really calling the stop code if the thread hadn't been started but simply noting the fact that it had been stopped. Then when start() was called the thread was actually started and then a true stop() was invoked on it immediately. Ironically, because the thread had been started the stop would trigger the VM operation and the stillborn flag would be set and so the checking of the stillborn flag (which hadn't worked for years) would now actually work! Unfortunately this solution has a race condition between the newly started thread and the call to stop() within the start code. If the new thread has already passed the stillborn check when it is stopped then it will execute until the pending exception is recognized and thrown. This means - as described in 4519200 - that now a thread that was stopped before start could actually execute some bytecodes rather than terminating immediately as it was supposed to do. So then 6562203 is opened because there are tests that don't expect this new behaviour. The fix for this is at two levels, within Thread and within hotspot: 1. In java.lang.Thread the fix for 4519200 is backed out. 2. In hotspot the newly started thread first checks to see if it is stillborn and immediately exits if that is the case. The stopThread method unconditionally sets the stillborn flag and then checks if the thread has been started to decide whether to invoke the VM operation. Notes: 1. Resurrection is handled in java.lang.Thread using threadState and so all comments/code pertaining to that in hotspot can be deleted. 2. Both Thread.stop and Thread.start are synchronized which prevents any race conditions between starts and stops. There is always a race between a stop() and the execution of the newly started thread, so even if stop() is called after start() the thread might still consider itself stillborn. This is fine in my view and is permitted by the spec through the words "has been started" which is more general than "has had the start() method invoked upon it". 3. We could just have java.lang.Thread.stop set the stillborn flag and then the only code needed in the VM would be the test of stillborn in the initial thread routine. However if we fixed it that way we'd need a "flag" day to sync up the library code with hotspot code. If the hotspot code were putback first then it would introduce new failures. This way the hotspot change can be putback without changing current failures, then when the Thread code is regressed the failures will stop. 4. For some reason hotspot distinguishes between a stop that throws ThreadDeath and stop(t) that throws an arbitrary throwable. Based on the specifications for the two methods there is no reason to treat the two cases differently. Based on the comment in the code this also has a history back in the 1.1/1.2 days but the reason for it seems lost (and unnecessary based on the spec). [Edited to correct #3. This putback alone won't fix the bug because the current Java code won't call JVM_StopThread if the thread isn't started.] * (#1 of 2): 2007-06-06 21:47:08 EST ###@###.### * Last Edit: 2007-06-07 16:01:43 EST ###@###.### Correction: The setting of still_born should only be done if the thread is not yet alive - this will ensure that it terminates immediately if it does get started. A start() followed by a stop() is not a "still-born" condition and so the flag should not be set. In that case the thread will stop by throwing the pending exception at the first execution point where pending asynchronous exceptions are recognized. This fixes stop-before-start, while preserving the inevitable race condition with stop-after-start. *** (#2 of 2): 2007-06-25 08:59:46 EST ###@###.###
09-07-2010
SUGGESTED FIX Set stillborn flag in JVM_StopThread if thread is not yet started, unconditionally test it in thread_main_inner and exit if set, and remove all other queries of the stillborn flag.
06-06-2007
EVALUATION See comments *** (#1 of 1): [ UNSAVED ] ###@###.###
06-06-2007