JDK-8195108 : Automate or provide dynamic thread registration for SMR
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 10
  • Priority: P3
  • Status: Resolved
  • Resolution: Duplicate
  • Submitted: 2018-01-15
  • Updated: 2019-05-28
  • Resolved: 2018-10-02
Related Reports
Duplicate :  
JDK-8209976 - Improve iteration over non-JavaThreads
Relates :  
JDK-8191798 - redo nested ThreadsListHandle to drop Threads_lock
Description
Attempting to transition to using SMR techniques ran into the following problem:

the _threads array inside my ThreadsListHandle stack local variable was destroyed / removed after removing the Threads_lock, which was covering the scope of using the ThreadsListHandle. 

It seems that it is not enough to take a SMR ThreadsListHandle for thread list interaction, but you also need to be registered as a thread that SMR will inspect when doing updates to the threads list (which it does under the exclusive Threads_lock).

Currently, this ���registration��� is done using Threads::do(), which means this is a static registration of threads to scan for SMR, that is, it is not a dynamic one.

We should figure out some means of providing automatic (ideal) and/or dynamic (ok) SMR thread registration.
Comments
JDK-8200076 is close to what i was looking for, thanks [~kbarrett] The NonJavaThread mechanism allows you to construct an NonJavaThread (obviously) without the need to explicitly export / register it for Threads::do() iteration. The registration is handled by the NonJavaThread constructor registering with the static list. I think we have removed most of the parts that were added as of JDK-8200374, but I think the exportation still remains (exporting the sampler thread). I think there is no longer any need for this external export, in that the sampler thread has now become a NonJavaThread. So from my perspective this can be closed, thanks [~dcubed]
02-10-2018
Taking a look at the current code base: - ThreadsSMRSupport::free_list() now calls: threads_do(&scan_cl); instead of "Threads::threads_do(&scan_cl)" - That results in a call to this code: // Apply the closure to all threads in the system. void ThreadsSMRSupport::threads_do(ThreadClosure *tc) { threads_do(tc, _java_thread_list); } - Which results in a call to this code: // Apply the closure to all threads in the system, with a snapshot of // all JavaThreads provided by the list parameter. void ThreadsSMRSupport::threads_do(ThreadClosure *tc, ThreadsList *list) { list->threads_do(tc); Threads::non_java_threads_do(tc); } The last function above handles all of the JavaThreads on the ThreadsList via "list->threads_do(tc)" and finally calls this: Threads::non_java_threads_do(tc) And that functions looks like this: // All NonJavaThreads (i.e., every non-JavaThread in the system). void Threads::non_java_threads_do(ThreadClosure* tc) { NoSafepointVerifier nsv(!SafepointSynchronize::is_at_safepoint(), false); for (NonJavaThread::Iterator njti; !njti.end(); njti.step()) { tc->do_thread(njti.current()); } } Based on the updated class hierarchy comment: open/src/hotspot/share/runtime/thread.hpp // Class hierarchy // - Thread // - JavaThread // - various subclasses eg CompilerThread, ServiceThread // - NonJavaThread // - NamedThread // - VMThread // - ConcurrentGCThread // - WorkerThread // - GangWorker // - GCTaskThread // - WatcherThread // - JfrThreadSampler we now have automatic Thread-SMR delete protocol cover for both JavaThread and NonJavaThread subclasses. All current subclasses are covered and if anyone adds a new subclass to either JavaThread or NonJavaThread, it will also be covered. To me, the key fix that made this an automatic and general solution to the problem is the use of a NonJavaThread::Iterator in the Threads::non_java_threads_do() function. That change was made: $ hg annot src/hotspot/share/runtime/thread.cpp | grep 'NonJavaThread::Iterator' 51548: NonJavaThread::Iterator::Iterator() : 51548: NonJavaThread::Iterator::~Iterator() { 51548: void NonJavaThread::Iterator::step() { 51548: for (NonJavaThread::Iterator njti; !njti.end(); njti.step()) { $ hg log -r 51548 changeset: 51548:35a6956f4243 user: kbarrett date: Tue Aug 28 16:04:54 2018 -0400 summary: 8209976: Improve iteration over non-JavaThreads So I'm inclined to close this bug as a duplicate of Kim's fix for JDK-8209976. Of course, I need to double check with [~mgronlun] (the submitter) and see if he believes this issue is already resolved.
19-09-2018
I think this was fixed by JDK-8200374, which added the JfrThreadSampler to the set of threads known to and visited by Threads::threads_do. And with JDK-8209976 the JfrThreadSampler (among others) is now a NonJavaThread, so will be automatically registered for visitation by Threads::threads_do and Threads::non_java_threads_do.
18-09-2018
If I understand the description correctly, you are saying that in order for a ThreadsListHandle to be honored by the Thread-SMR protocol, the thread that created the ThreadsListHandle has to be visible via the Threads::threads_do() mechanism. So ThreadsList objects are deleted by this function: src/hotspot/share/runtime/threadSMR.cpp: ThreadsSMRSupport::free_list() Key snippets of code are: // Gather a hash table of the current hazard ptrs: ThreadScanHashtable *scan_table = new ThreadScanHashtable(hash_table_size); ScanHazardPtrGatherThreadsListClosure scan_cl(scan_table); Threads::threads_do(&scan_cl); The threads_do() call gathers all the active hazard ptrs and this block: // Walk through the linked list of pending freeable ThreadsLists // and free the ones that are not referenced from hazard ptrs. ThreadsList* current = _to_delete_list; ThreadsList* prev = NULL; ThreadsList* next = NULL; bool threads_is_freed = false; while (current != NULL) { next = current->next_list(); if (!scan_table->has_entry((void*)current)) { // This ThreadsList is not referenced by a hazard ptr. if (prev != NULL) { prev->set_next_list(next); } if (_to_delete_list == current) { _to_delete_list = next; } log_debug(thread, smr)("tid=" UINTX_FORMAT ": ThreadsSMRSupport::free_list: threads=" INTPTR_FORMAT " is freed.", os::current_thread_id(), p2i(current)); if (current == threads) threads_is_freed = true; delete current; if (EnableThreadSMRStatistics) { _java_thread_list_free_cnt++; _to_delete_list_cnt--; } } else { prev = current; } current = next; } goes through the _to_delete_list and frees the ThreadsList objects that are not active hazard ptrs as found via the Threads::threads_do() call above. So I agree that if Threads::threads_do() does not see a thread that created the ThreadsListHandle, then that ThreadsListHandle will not actually participate in the Thread-SMR protocols. So how is it that the thread is not seen by Threads::threads_do()? What kind of thread is this and how was it created?
16-01-2018