JDK-8240588 : _threadObj cannot be used on an exiting JavaThread.
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 15
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-03-05
  • Updated: 2020-05-27
  • Resolved: 2020-05-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 15
15 b24Fixed
Related Reports
Relates :  
JDK-8241403 - JavaThread::get_thread_name() should be ThreadSMR-aware
Relates :  
JDK-8244997 - Convert the JavaThread::_threadObj oop to use OopStorage
Description
We never clear a JavaThread's _threadObj.
If JavaThread is exiting the oop may be rubbish.
I think we should clear the field before removing us from the master threadslist.
Otherwise if you have a ThreadsList snapshot, you must always check is_exiting before touching the oop, which feels cumbersome.
But clearing may force you to always null check it instead.

Maybe we must try to keep the oop alive, which I really don't want to.

Crashing thread:
#9  <signal handler called>
#10 RawAccessBarrier<64ul>::load_internal<64ul, long> (addr=0x83a85000640)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:729
#11 RawAccessBarrier<64ul>::load<long> (addr=0x83a85000640) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:344
#12 AccessInternal::PreRuntimeDispatch::load<544848ul, long> (addr=0x83a85000640)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:734
#13 AccessInternal::PreRuntimeDispatch::load_at<544848ul, long> (offset=<optimized out>, base=...)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:767
#14 AccessInternal::PreRuntimeDispatch::load_at<540752ul, long> (offset=<optimized out>, base=...)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:776
#15 AccessInternal::load_at<524288ul, long> (offset=<optimized out>, base=...)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:1190
#16 AccessInternal::LoadAtProxy<524288ul>::operator long<long>() const (this=0x7f91249323e0)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/accessBackend.hpp:1314
#17 oopDesc::long_field (this=<optimized out>, offset=<optimized out>) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/oop.inline.hpp:282
--Type <RET> for more, q to quit, c to continue without paging--
#18 0x00007f93d58e53be in java_lang_Thread::thread_id (java_thread=...)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/oopsHierarchy.hpp:101
#19 0x00007f93d61f020f in ThreadsList::find_JavaThread_from_java_tid (this=0x7f90b83919b0, java_tid=99284)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/oopsHierarchy.hpp:86
#20 0x00007f93d5da9fd5 in do_thread_dump (dump_result=dump_result@entry=0x7f91249326a0, ids_ah=..., num_threads=num_threads@entry=352,
    max_depth=max_depth@entry=-1, with_locked_monitors=with_locked_monitors@entry=false, with_locked_synchronizers=with_locked_synchronizers@entry=false,
    __the_thread__=0x7f93d0a4f8b0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/threadSMR.hpp:285
#21 0x00007f93d5daaaab in jmm_GetThreadInfo (env=<optimized out>, ids=<optimized out>, maxDepth=-1, infoArray=<optimized out>)
    at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/services/management.cpp:1112

The JavaThread whose oop we are trying to use:
#0  0x00007f93d6b3674d in __lll_lock_wait () from /lib64/libpthread.so.0
#1  0x00007f93d6b2fcf9 in pthread_mutex_lock () from /lib64/libpthread.so.0
#2  0x00007f93d5e81d28 in os::PlatformMutex::lock (this=0x7f93d0063fd0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/os/posix/os_posix.hpp:279
#3  Mutex::lock_contended (this=this@entry=0x7f93d0063fc0, self=self@entry=0x7f8f40026da0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/mutex.cpp:96
#4  0x00007f93d5e81efe in Mutex::lock (this=0x7f93d0063fc0, self=0x7f8f40026da0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/mutex.cpp:109
#5  0x00007f93d61f2d37 in MutexLocker::MutexLocker (flag=Mutex::_safepoint_check_flag, mutex=0x7f93d0063fc0, this=<synthetic pointer>) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/mutexLocker.hpp:202
#6  ThreadsSMRSupport::smr_delete (thread=<optimized out>) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/threadSMR.cpp:965
#7  0x00007f93d61e26c0 in Thread::call_run (this=this@entry=0x7f8f40026da0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/thread.cpp:409
#8  0x00007f93d5f14856 in thread_native_entry (thread=0x7f8f40026da0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/os/linux/os_linux.cpp:789
#9  0x00007f93d6b2d4c0 in start_thread () from /lib64/libpthread.so.0
#10 0x00007f93d6a55163 in clone () from /lib64/libc.so.6
Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/9c608830327b User: dholmes Date: 2020-05-14 02:31:54 +0000
14-05-2020
Regression test using WhiteBox was devised. Here is sample failure output: [2020-05-13T00:46:35,845Z] STDOUT: [2020-05-13T00:46:35,845Z] WB_CheckThreadObjOfTerminatingThread: target thread is protected [2020-05-13T00:46:35,845Z] GCThread waiting ... [2020-05-13T00:46:35,845Z] Target is terminating [2020-05-13T00:46:35,845Z] WB_CheckThreadObjOfTerminatingThread: target thread is terminated [2020-05-13T00:46:35,845Z] WB_CheckThreadObjOfTerminatingThread: GC has been initiated - checking threadObj: [2020-05-13T00:46:35,845Z] WB_CheckThreadObjOfTerminatingThread: successful comparison on iteration 0 [2020-05-13T00:46:35,845Z] GCThread running ... [2020-05-13T00:46:35,845Z] WB_CheckThreadObjOfTerminatingThread: failed comparison on iteration 1 [2020-05-13T00:46:35,845Z] STDERR: [2020-05-13T00:46:35,845Z] java.lang.RuntimeException: Target thread oop has changed! [2020-05-13T00:46:35,845Z] at sun.hotspot.WhiteBox.checkThreadObjOfTerminatingThread(Native Method) [2020-05-13T00:46:35,845Z] at ThreadObjAccessAtExit.main(ThreadObjAccessAtExit.java:117) [2020-05-13T00:46:35,845Z] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) [2020-05-13T00:46:35,845Z] at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64) [2020-05-13T00:46:35,845Z] at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) [2020-05-13T00:46:35,845Z] at java.base/java.lang.reflect.Method.invoke(Method.java:564) [2020-05-13T00:46:35,845Z] at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) [2020-05-13T00:46:35,845Z] at java.base/java.lang.Thread.run(Thread.java:832) [2020-05-13T00:46:35,845Z]
13-05-2020
This actually turned out to be far simpler than envisaged once I realized I needed to just track the threads not the other ThreadsLists. A simple enhancement to ThreadsSMRSupport fixes the potential problem: // Once a JavaThread has removed itself from the main ThreadsList it is // no longer visited by GC. To ensure that thread's threadObj() oop remains // valid while the thread is still accessible from a ThreadsListHandle we // maintain a special list of exiting threads: // - In remove() we add the exiting thread to the list (under the Threads_lock). // - In wait_until_not_protected() we remove it from the list (again under the // Threads_lock). // - Universe::oops_do walks the list (at a safepoint so VMThread holds // Threads_lock) and visits the _threadObj oop of each JavaThread Testing with instrumentation shows that there is very rarely a thread in the exiting threads list when the GC runs. Trying to come up with a regression test will be far harder than the fix I think.
06-05-2020
I'm going to try a simple scheme that exposes the secondary threadLists to GC via a simple linked-list with a global head.
30-04-2020
After discussions with Kim it seems there is no solution that would allow the exiting thread to safely stash the _threadObj oop and then clear it later, as all such solutions require that it be cleared before the exiting thread calls on_thread_detach. A possible alternative here would be for the exiting thread to request the serviceThread do the deletion on its behalf e.g. we could use the JNI Global ref solution and have the serviceThread destroy it. Of course we still need clear communication lines between the exiting thread and the serviceThread.
21-04-2020
JVM TI does seem fine because either we are at a safepoint and have checked is_exiting, or we immediately handlize and return a JNI reference. However in general it is tedious to determine the safety of each usage and even if determined safe we then have to add comments everywhere to say "this call to threadObj() is safe because ...". I don't really see this as a satisfactory solution at all.
20-04-2020
The comment regarding JvmtiExport::cv_external_thread_to_JavaThread was incorrect. While it does return the oop it is actually returning the oop associated with a jthread that was passed in, so there is already an external guard on ensuring that the thread oop is valid
20-04-2020
I don't think you can use Handles like that. There are no setter operations on Handles so you can't pass a pointer or reference to an "empty" Handle and then have the called method set the oop inside the Handle.
15-04-2020
Yes thanks for looking into it David. If we instead of checking is_exiting in those places, we could check threadObj after handlize it, JVM TI ~should~ be fine. E.g. instead of parameter "oop * thread_oop_p" we take address to a Handle where we can put it if the thread is considered alive.
15-04-2020
The fan out of this is larger than I had anticipated. Not only do we have all uses of threadObj() to check, but also the clients of APIs like JvmtiExport::cv_external_thread_to_JavaThread which can also return the thread_oop as an out parameter. All these cases have to immediately wrap the oop in a Handle to protect it (or return a Handle from the lower API). This is very intrusive to say the least. One of the key goals of ThreadSMR was to ensure there was a safe way for things like JVM TI to get a reference to a thread and not have it terminate and get deleted while being examined. We fixed that with regards to the direct thread state, but missed ensuring the threadObj() oop was safe. I would really like a solution that keeps the oop safe to access instead of having to make all client code robust.
15-04-2020
Some (most ?) code already has to check is_exiting anyway. But I'll see what is needed for each call site. The cleanest/simplest code would still be if the threadObj was guaranteed valid if you have a reference to the JavaThread associated with it. I'm disappointed I can't use JNI Global ref as intended. As for get_thread_name() ... I think with virtual threads on the horizon it will make little sense to try and maintain the name in the VM as well as in Java code.
14-04-2020
Yes, is_exiting should be safe, but as I said null checking the oop is easier to follow and nicer code (if we clear it at correct place). Maybe get_thread_name() should be backed with a native resource instead, keeping the name of the thread until it dies seem like a good thing?
14-04-2020
Turns out the problem here is more complex than thought. In addition to the _threadObj of a thread becoming unsafe after the thread has been removed from the main ThreadsList, it seems that an exiting thread is not permitted to interact with oops after the call to barrier_set()->on_thread_detach(this)! This means that the attempt to use a JNI Global reference fails because deleting a global reference still interacts with the current thread's internals in relation to oops access. I found this somewhat counter-intuitive and seems a limitation of our implementation, but nevertheless it means I would need to delete the global ref before on_thread_detach, which defeats the whole point of using the global ref! Note that this also means that we cannot NULL _threadObj whilst under the ThreadsLock in Threads::remove but must again do it before on_thread_detach. On relation to checking is_exiting() I would argue that it is safe to check that, even though it could change as soon as being read, because for the threadObj to become invalid a GC must occur, which can't happen unless the thread that is checking is_exiting hits a safepoint. So as long as we handleize threadObj() straight-away it should be fine. Also note that we have a direct unsafe access to the threadObj immediately after Threads::remove is called, in the logging statement that calls get_thread_name(). Any crash involving the current thread will also unsafely access threadObj via get_thread_name(). These will also need to be fixed.
14-04-2020
Ideally I would like to suggest that all threads on any threads-list are visited by oops_do so that the threadObj, and any Handle still held by the thread, are kept valid. In practice this seems impractical given the way the additional threads-lists are effectively thread-locals. We would need to visit each thread-local thread-list as part of that thread's oops_do, but that will easily lead to cycles. While there would be a way to traverse the graph without cycles the overhead and complexity suggests we not look at this particular solution. Another possible approach I have prototyped is to actually keep the thread oop in a JNI global handle, so that it is always valid. This seems to work functionally but also requires the addition of set_threadObj(NULL) to delete the global handle (as part of thread deletion) to avoid a leak. It increases the JavaThread footprint (as we still need the raw oop field for by the intrinsics for java.lang.Thread::currentThread(). It also suggests modifying many of the callers of threadObj() so that we don't go from JNI-Handle -> oop -> local Handle. Otherwise as has been suggested above we clear _threadObj whilst still under the Threads_lock and we then have to check all call-sites to ensure they check for is_null() and properly Handle-ize the oop before any GC can occur (which should already be the case anyway).
09-04-2020
ILW = MMM = P3
10-03-2020
All these races should not be possible i.e. we should not allow them to be possible. ThreadsLists were added to ensure that the JavaThread memory could not be deleted whilst still in use by another thread (without always needing to hold Threads_lock). But in adding ThreadsLists we have allowed the threadObj() to racily become garbage! That is definitely an oversight and a mistake IMO. All active ThreadsLists should be stable from a GC perspective.
09-03-2020
When holding a threadslist snapshot, any thread may still remove it self from the master list. If not on master list GC will ignore you. (maybe that is a design mistake, as David suggested) Which means even if you check is_existing the oop may become rubbish afterwards, if you don't save it before checking is_exiting and keep it alive. What I have added now so my Kitchensink don't crash (trying to repro another bug): HandleMark hm(current); Handle th_obj(current, target->threadObj()); if (th_obj.is_null()) { // BAIL_OUT } // Now we are fine, current thread will keep this oop alive. And I clear it in ensure_join (where the bug-free place to clear, I'm not sure of): thread->set_threadObj(NULL); But there are many uses of threadObj(), most of them are already okay, e.g. target suspended, in safepoint, etc..., but still every place needs to carefully looked at. If don't clear it and instead use is_exiting we still need to similar: HandleMark hm(current); Handle th_obj(current, target->threadObj()); if (target->is_exiting()) { // BAIL_OUT } // Now we are fine, current thread will keep this oop alive. So it feel more natural to check if oop is non-null. If we don't read and save the oop before: if (target->is_exiting()) { // Bail-out } // target now sets exiting and removes itself from master list target->threadObj(); // This is or can become rubbish This means clearing it after is_exiting doesn't really help much, since the flag may be set just after you read it. So therefore I suggest just clearing it and take the pain with adding proper null checks. EDIT: Since we need safepoint to happen below should not be needed: Btw, another reason not to use is_exiting is because we need a storeload fence between storing the oop into our handle-area and reading the is_exiting.
06-03-2020
Okay so ... Assuming it is not an option to add in oops_do processing of all JavaThreads on any ThreadsList (the omission of which seems an oversight to me) then the fix here is two-fold: a) clear threadObj() at an appropriate point after is_exiting() returns true; and b) ensure any code that examines threadObj() and checks is_exiting(), does the is_exiting() check first. Maybe we can add an assert to threadObj() to ensure it isn't called when is_exiting() is true?
06-03-2020
Adding a !thread->is-exiting() test to that code wouldn't hurt my feelings at all. :-) > I assume the oop is cleared after is_exiting() will return true. If you mean in the baseline, then the oop isn't cleared. If you mean in this location that I mumbled about above: src/hotspot/share/runtime/thread.cpp: // Make sure that safepoint code disregard this thread. This is needed since // the thread might mess around with locks after this point. This can cause it // to do callbacks into the safepoint code. However, the safepoint code is not aware // of this thread since it is removed from the queue. p->set_terminated_value(); } // unlock Threads_lock then, is_exiting() will return 'true' before this point. That happens here: src/hotspot/share/runtime/thread.cpp: void JavaThread::exit(bool destroy_vm, ExitType exit_type) { <snip> // We have notified the agents that we are exiting, before we go on, // we must check for a pending external suspend request and honor it // in order to not surprise the thread that made the suspend request. while (true) { { MutexLocker ml(SR_lock(), Mutex::_no_safepoint_check_flag); if (!is_external_suspend()) { set_terminated(_thread_exiting); ThreadService::current_thread_exiting(this, is_daemon(threadObj())); break; } // Implied else: // Things get a little tricky here. We have a pending external // suspend request, but we are holding the SR_lock so we // can't just self-suspend. So we temporarily drop the lock // and then self-suspend. } ThreadBlockInVM tbivm(this); java_suspend_self(); // We're done with this suspend request, but we have to loop around // and check again. Eventually we will get SR_lock without a pending // external suspend request and will be able to mark ourselves as // exiting. } // no more external suspends are allowed at this point The call to set_terminated(_thread_exiting) above is what causes is_exiting() to return true. Once we gotten thru the final check for external suspend dance without actually self-suspending, we can set _thread_exiting...
06-03-2020
So in: oop tobj = thread->threadObj(); // Ignore the thread if it hasn't run yet, has exited // or is starting to exit. if (tobj != NULL && java_tid == java_lang_Thread::thread_id(tobj)) { we should do the thread->is-exiting() test first so that we don't grab the oop from an exiting thread. I assume the oop is cleared after is_exiting() will return true.
05-03-2020
I'm looking at this part of the crashing stack: #18 0x00007f93d58e53be in java_lang_Thread::thread_id (java_thread=...) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/oopsHierarchy.hpp:101 #19 0x00007f93d61f020f in ThreadsList::find_JavaThread_from_java_tid (this=0x7f90b83919b0, java_tid=99284) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/oops/oopsHierarchy.hpp:86 #20 0x00007f93d5da9fd5 in do_thread_dump (dump_result=dump_result@entry=0x7f91249326a0, ids_ah=..., num_threads=num_threads@entry=352, max_depth=max_depth@entry=-1, with_locked_monitors=with_locked_monitors@entry=false, with_locked_synchronizers=with_locked_synchronizers@entry=false, __the_thread__=0x7f93d0a4f8b0) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/runtime/threadSMR.hpp:285 #21 0x00007f93d5daaaab in jmm_GetThreadInfo (env=<optimized out>, ids=<optimized out>, maxDepth=-1, infoArray=<optimized out>) at /home/rehn/source/jdk/vanilla-jdk/open/src/hotspot/share/services/management.cpp:1112 This is a jmm_GetThreadInfo() call which is calling do_thread_dump() which uses ThreadsList::find_JavaThread_from_java_tid (). This looks to be this code: src/hotspot/share/services/management.cpp: static void do_thread_dump(ThreadDumpResult* dump_result, typeArrayHandle ids_ah, // array of thread ID (long[]) int num_threads, int max_depth, bool with_locked_monitors, bool with_locked_synchronizers, TRAPS) { // no need to actually perform thread dump if no TIDs are specified if (num_threads == 0) return; // First get an array of threadObj handles. // A JavaThread may terminate before we get the stack trace. GrowableArray<instanceHandle>* thread_handle_array = new GrowableArray<instanceHandle>(num_threads); { // Need this ThreadsListHandle for converting Java thread IDs into // threadObj handles; dump_result->set_t_list() is called in the // VM op below so we can't use it yet. ThreadsListHandle tlh; for (int i = 0; i < num_threads; i++) { jlong tid = ids_ah->long_at(i); JavaThread* jt = tlh.list()->find_JavaThread_from_java_tid(tid); oop thread_obj = (jt != NULL ? jt->threadObj() : (oop)NULL); instanceHandle threadObj_h(THREAD, (instanceOop) thread_obj); thread_handle_array->append(threadObj_h); } } // Obtain thread dumps and thread snapshot information VM_ThreadDump op(dump_result, thread_handle_array, num_threads, max_depth, /* stack depth */ with_locked_monitors, with_locked_synchronizers); VMThread::execute(&op); } and that call to find_JavaThread_from_java_tid() lands here: src/hotspot/share/runtime/threadSMR.cpp: JavaThread* ThreadsList::find_JavaThread_from_java_tid(jlong java_tid) const { ThreadIdTable::lazy_initialize(this); JavaThread* thread = ThreadIdTable::find_thread_by_tid(java_tid); if (thread == NULL) { // If the thread is not found in the table find it // with a linear search and add to the table. for (uint i = 0; i < length(); i++) { thread = thread_at(i); oop tobj = thread->threadObj(); // Ignore the thread if it hasn't run yet, has exited // or is starting to exit. if (tobj != NULL && java_tid == java_lang_Thread::thread_id(tobj)) { MutexLocker ml(Threads_lock); // Must be inside the lock to ensure that we don't add a thread to the table // that has just passed the removal point in ThreadsSMRSupport::remove_thread() if (!thread->is_exiting()) { ThreadIdTable::add_thread(java_tid, thread); return thread; } } } } else if (!thread->is_exiting()) { return thread; } return NULL; } This is relevant code block: oop tobj = thread->threadObj(); // Ignore the thread if it hasn't run yet, has exited // or is starting to exit. if (tobj != NULL && java_tid == java_lang_Thread::thread_id(tobj)) { If 'tobj' is non-NULL, then this code is assuming that it is a valid oop and can be passed to the java_lang_Thread::thread_id() call which is going to try and fetch the thread_id field value from the java.lang.Thread object. If the _threadObj oop is bad, then we run the risk of a BOOM! Update: There is logic in the M&M code to keep the info stored in a ThreadDumpResult alive via oops_do() calls. I suspect that there is a window in the following sequence: - M&M code creates ThreadsList which includes the target thread - The target thread is exiting so it removes itself from the system's ThreadsList. - A GC happens, but the _threadObj field isn't refreshed because it is not on the system's ThreadsList. The _threadObj field isn't refreshed via the M&M code either because the JavaThread isn't mentioned in the ThreadDumpResult yet either. So the exiting JavaThread has slipped thru the cracks. Of course, this is pure theory on my part.
05-03-2020
So the threads are visited by GC via: src/hotspot/share/runtime/thread.cpp: void Threads::oops_do(OopClosure* f, CodeBlobClosure* cf) { ALL_JAVA_THREADS(p) { p->oops_do(f, cf); } VMThread::vm_thread()->oops_do(f, cf); } The ALL_JAVA_THREADS macro causes JavaThread::oops_do() to be called on all JavaThreads on the system's ThreadsList. So once a JavaThread has been removed from the system's ThreadsList, there isn't anything to call JavaThread::oops_do() on it so there isn't anything to keep that oop alive. Unless, of course, I missing some other JavaThread visiting path like from M&M code... dunno... It's bad hygiene to not clear the _threadObj field after the JavaThread has been removed from the system's ThreadsList so we can look at clearing that field as part of Threads::remove() under the protection of the Threads_lock. I would not clear the field before the JavaThread is removed from the system's ThreadsList. Update: I'm thinking that clearing the _threadObj field via a set_threadObj(NULL) would be good right after the set_terminated_value() call here: src/hotspot/share/runtime/thread.cpp: // Make sure that safepoint code disregard this thread. This is needed since // the thread might mess around with locks after this point. This can cause it // to do callbacks into the safepoint code. However, the safepoint code is not aware // of this thread since it is removed from the queue. p->set_terminated_value(); } // unlock Threads_lock
05-03-2020
> In many cases when you have a ThreadsList you need to check is_exiting > anyway to ensure you aren't including a thread that shouldn't be there. I would phrase this a little different. When you are walking a ThreadsList, you should check each thread's is_exiting() return status to see if the JavaThread has indicated that it is exiting. In that case, you may not want to include that JavaThread in your processing loop. It isn't a case that the JavaThread "shouldn't be there". It's a case that the JavaThread has marked itself as exiting and has not yet removed itself from the Threadslist.
05-03-2020
In many cases when you have a ThreadsList you need to check is_exiting anyway to ensure you aren't including a thread that shouldn't be there.
05-03-2020
I'm not clear why we would need to clear threadObj()? As long as the JavaThread is reachable via a ThreadsList we may want to access the threadObj(). And as long as the JavaThread is accessible the threadObj oop should be being kept alive by JavaThread::oops_do(). Have we overlooked that?
05-03-2020