RFR: 8285794: AsyncGetCallTrace might acquire a lock via JavaThread::thread_from_jni_environment [v2]

[David Holmes]

On Thu, 28 Apr 2022 11:38:32 GMT, Johannes Bechberger <duke at openjdk.java.net> wrote:

>> Calling JavaThread::thread_from_jni_environment for a terminated thread in AsyncGetCallTrace might cause the acquisition of a lock, making AsyncGetCallTrace non-signal-safe. 
>> 
>> AsyncGetCallTrace can only be called for the current threads (there are asserts for that), therefore using JavaThread::current directly and checking the termination status is semantically equivalent.
>
> Johannes Bechberger has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use Thread::current_or_null_safe() and add comments

Changes requested by dholmes (Reviewer).

src/hotspot/share/prims/forte.cpp line 35:

> 33: #include "runtime/frame.inline.hpp"
> 34: #include "runtime/javaCalls.hpp"
> 35: #include "runtime/thread.hpp"

You don't need this as you already include the thread.inline.hpp which has to include thread.hpp.

src/hotspot/share/prims/forte.cpp line 571:

> 569:   Thread* raw_thread = Thread::current_or_null_safe();
> 570: 
> 571:   if (trace->env_id == NULL || raw_thread == NULL || !raw_thread->is_Java_thread() || ((JavaThread*)raw_thread)->is_exiting()) {

use `rawThread->as_JavaThread()` not a plain cast.

src/hotspot/share/prims/forte.cpp line 577:

> 575:   }
> 576: 
> 577:   JavaThread* thread = (JavaThread*)raw_thread;

use `rawThread->as_JavaThread()` not a plain cast. You could also incorporate this into the earlier code so you don't do it twice.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8446