8214097: Rework thread initialization and teardown logic #2411
Conversation
|
👋 Welcome back sunny868! A progress list of the required criteria for merging this PR into |
openjdk
bot
changed the title
|
This backport pull request has now been updated with issue from the original commit. |
|
/approval request Fix Request jdk11u |
|
Hi, |
|
@sunny868 Why are you backporting this patch to OpenJDK 11u? It's an intrusive patch that changes thread initialization which isn't suitable for backport at this stage in the lifecycle of JDK 11. There must be a very good reason to even consider it. |
runtime/InternalApi/ThreadCpuTimesDeadlock.java can trigger exception occasionally. I traced the cause: // src/hotspot/os/linux/os_linux.cpp
// be called by com.sun.jmx.mbeanserver.JmxMBeanServer
6171 static jlong fast_cpu_time(Thread *thread) {
6172 clockid_t clockid;
6173 int rc = os::Linux::pthread_getcpuclockid(thread->osthread()->pthread_id(),
6174 &clockid); // thread->osthread() maybe NULL or pthread_id() is 0.
6175 if (rc == 0) {
6176 return os::Linux::fast_thread_cpu_time(clockid);
6177 } else {
6178 // It's possible to encounter a terminated native thread that failed
6179 // to detach itself from the VM - which should result in ESRCH.
6180 assert_status(rc == ESRCH, rc, "pthread_getcpuclockid failed");
6181 return -1;
6182 }
6183 }
Here. Line 6173 // share/runtime/thread.cpp
1309 NonJavaThread::NonJavaThread() : Thread(), _next(NULL) {
1310 // Add this thread to _the_list.
1311 MutexLockerEx lock(NonJavaThreadsList_lock, Mutex::_no_safepoint_check_flag);
1312 _next = _the_list._head;
1313 OrderAccess::release_store(&_the_list._head, this); // Do early. It should be done after thread state is RUNNED
1314 }
I see this patch do os::create_thread(Thread* thread ...) {
thread->set_osthread(osthread);
pthread_create(&tid, &attr, (void* (*)(void*)) thread_native_entry, thread);
osthread->set_pthread_id(tid);
wait until osthread state equal INITIALIZED.
}
thread_native_entry(Thread *thread) {
osthread->set_state(INITIALIZED);
wait until os::start_thread() // set thread state RUNNED
thread->call_run(); // call NonJavaThread::pre_run(), add thread to _the_list
}
|
Thank you for the reminder. I haven't tested the debug mode locally, so it seems that I still need relevant patch support. |
| // and that happens just before Thread::current is set. No other thread | ||
| // can attach as the VM is not created yet, so they can't execute this code. | ||
| // If the main thread creates other threads before the barrier set that is an error. | ||
| assert(Thread::current_or_null() == NULL, "creating thread before barrier set"); |
There was a problem hiding this comment.
Failed here in fastdebug mode, I'm not sure if this is correct
I'm sorry, but this isn't a good enough reason to accept the risk of doing such an invasive backport. |
|
@sunny868 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@sunny868 This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the |
runtime/InternalApi/ThreadCpuTimesDeadlock.java can trigger exception occasionally on jdk11u fastdebug, the reason is that when
fast_cpu_time(Thread *thread)is called, a non-java-thread state is ALLOCATED, so set_osthread (osthread) or osthread ->set_pthread_id (tid) is not yet completed.so need backport this patch.
GHA and tier2-3 locally has finish, the failed items are not related to this patch.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk11u-dev.git pull/2411/head:pull/2411$ git checkout pull/2411Update a local copy of the PR:
$ git checkout pull/2411$ git pull https://git.openjdk.org/jdk11u-dev.git pull/2411/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 2411View PR using the GUI difftool:
$ git pr show -t 2411Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk11u-dev/pull/2411.diff
Webrev
Link to Webrev Comment