On Tue, 17 May 2022 05:38:01 GMT, David Holmes <dhol...@openjdk.org> wrote:

>> Jaroslav Bachorik has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Restore the original find_blob behavior regarding dead blobs
>
> src/hotspot/share/prims/forte.hpp line 53:
> 
>> 51:   ASGCTMark() : ASGCTMark(JavaThread::current()) {}
>> 52:   ~ASGCTMark() {
>> 53:     JavaThread::current()->set_in_asgct(false);
> 
> You can't call `JavaThread::current()` in any of this code as it is not safe 
> from a signal handling context. (There is an existing use in ASGCT that is 
> also unsafe.) I suggest not having the no-arg constructor and require the 
> current JavaThread, or null, to be passed in (having already been safely 
> obtained by the caller). You can then assert using 
> `Thread::current_or_null_safe()`.
> 
> Personally I find the ASGCTMark complete overkill for this situation as there 
> is only ever going to be a single use - sorry @tstuefe - it is just 
> complicating things IMO.

Ok, I fixed the `ASGCTMark` to be safe to use from a signal handler.

I have no strong opinion about whether we should keep it or not - it makes the 
code in `forte.cpp` slightly more resilient in case of further modifications 
for the price of more complexity introduced by the mark class 🤷

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

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

Reply via email to