On Fri, 15 Oct 2021 18:27:43 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> src/hotspot/share/runtime/thread.cpp line 1771: >> >>> 1769: guarantee(Thread::is_JavaThread_protected_by_my_ThreadsList(this), >>> 1770: "missing ThreadsListHandle in calling context."); >>> 1771: if (is_exiting()) { >> >> This seems an unrelated change in behaviour ?? > > So suspend_thread and resume thread's caller already takes a > ThreadsListHandle so this is unnecessary and never happens. > This seems an unrelated change in behaviour ?? Actually this is equivalent code. The baseline code does this: ThreadsListHandle tlh; if (!tlh.includes(this)) { log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " not on ThreadsList, no suspension", p2i(this)); return false; } What that code means is: if `this` thread does not appear in the newly created ThreadsListHandle's list, then `this` thread has called `java_suspend()` after `this` thread has exited. That's the only way that `this` thread could be missing from a newly created ThreadsListHandle's list. All I've done here is get rid of the ThreadsListHandle, verify that the calling context is protecting `this` and switch to an `is_exiting()` call. > So suspend_thread and resume thread's caller already takes a ThreadsListHandle > so this is unnecessary and never happens. I presume @coleenp is saying that the `is_exiting()` check is not necessary. That might be true, it might not be true. I was trying to create equivalent code without creating a nested ThreadsListHandle here and I think I've done that. I actually think it might be possible for a JVM/TI event handler to fire after the thread has set is_exiting() and for that event handler to call SuspendThread() which could get us to this point. ------------- PR: https://git.openjdk.java.net/jdk/pull/4677