RFR: 8241296: Segfault in JNIHandleBlock::oops_do()

[Stefan Karlsson]

Hi Andrew,

I think the fix is fine.

However, it's also seems to go against some other parts of the code, 
that tries to setup threads and add them to the thread lists *after* the 
handles have been created:

attach_current_thread:
   // This thread will not do a safepoint check, since it has
   // not been added to the Thread list yet.
   { MutexLocker ml(Threads_lock);
     // This must be inside this lock in order to get FullGCALot to work 
properly, i.e., to
     // avoid this thread trying to do a GC before it is added to the 
thread-list
     thread->set_active_handles(JNIHandleBlock::allocate_block());
     Threads::add(thread, daemon);
   }

Or without a safepoint between the setting of the _active_handles and 
the adding of the thread to the thread list when going through the 
normal pre_run/run setup. Or did I miss a safepoint somewhere?

I do see this code in JavaThread::exit:

   if (active_handles() != NULL) {
     JNIHandleBlock* block = active_handles();
     set_active_handles(NULL);
     JNIHandleBlock::release_block(block);
   }
...
   if (JvmtiEnv::environments_might_exist()) {
     JvmtiExport::cleanup_thread(this);
   }
...

   Threads::remove(this, daemon);

where cleanup_threads take a lock *with* a safepoint check, allowing GCs 
to run and exposing a NULL _active_handle.

Would you mind sharing some extra info? For example the stack trace of 
the scanned thread, and / or flags used to provoke this? I would like to 
know why we haven't seen this before.

Thanks,
StefanK

On 2020-03-19 15:58, Andrew Haley wrote:
> We're seeing intermittent SEGVs in JDKs with some newer GCC versions
> and combinations of options. It turns out that it's a pretty trivial
> error which has never been noticed before.
> 
> Thread::oops_do() does this:
> 
> void Thread::oops_do(OopClosure* f, CodeBlobClosure* cf) {
>    active_handles()->oops_do(f);
> 
> However, there is a window while a Thread is being constructed when
> active_handles() is NULL. GC can occur during this time period, and
> it's a matter of luck that we haven't seen this crash before.
> 
> http://cr.openjdk.java.net/~aph/8241296/
> 
> OK to push?
>