[9] RFR(S): 8075214: SIGSEGV in nmethod sweeping (original) (raw)

Mikael Gerdin mikael.gerdin at oracle.com
Wed Mar 25 14:51:12 UTC 2015

Hi Tobias,

Handling Java-level exceptions in the C++ code is fairly convoluted. Stare in horror at the macros in exceptions.hpp and see my comments below.

On 2015-03-25 14:53, Tobias Hartmann wrote:

Hi,

please review the following patch. https://bugs.openjdk.java.net/browse/JDK-8075214 http://cr.openjdk.java.net/~thartmann/8075214/webrev.00/

818 Klass* k = SystemDictionary::resolve_or_fail(vmSymbols::java_lang_Thread(), true, THREAD);

This THREAD and a lot of the following ones need to be CHECK_NULL If any function throws a Java exception (such as OOME) then you cannot continue and must return a failure to the caller.

I realize that at this exact point resolving the Thread class should not fail, but if it does for some strange reason you will SEGV a few lines below. You could make this a CATCH, which will crash the VM on an unexpected exception, which may be ok for a white box test function.

819 instanceKlassHandle klass(THREAD, k); 820 instanceHandle thread_oop = klass->allocate_instance_handle(THREAD);

You can get OOME here and need to bail out if thread_oop is null due to an allocation failure, change this THREAD to CHECK_NULL

821 822 Handle name = java_lang_String::create_from_str("WB Sweeper thread", THREAD);

Allocating memory for the String can OOME, replace with CHECK_NULL

823 824 // Initialize thread_oop to put it into the system threadGroup 825 Handle thread_group(THREAD, Universe::system_thread_group()); 826 JavaValue result(T_VOID); 827 JavaCalls::call_special(&result, 828 thread_oop, 829 klass, 830 vmSymbols::object_initializer_name(), 831 vmSymbols::threadgroup_string_void_signature(), 832 thread_group, 833 name, 834 THREAD);

I'd pass CHECK_NULL here as well, since it's unclear if Thread:: can throw any exception.

835 836 KlassHandle group(THREAD, SystemDictionary::ThreadGroup_klass()); 837 JavaCalls::call_special(&result, 838 thread_group, 839 group, 840 vmSymbols::add_method_name(), 841 vmSymbols::thread_void_signature(), 842 thread_oop, // ARG 1 843 THREAD);

Adding to the thread group can cause an OOME (the Arrays.copyOf allocates a new array), add CHECK_NULL.

844 845 { 846 MutexLocker mu(Threads_lock); 847 848 // create sweeper thread w/ custom entry -- one iteration instead of loop 849 CodeCacheSweeperThread* sweeper_thread = new CodeCacheSweeperThread(); 850 sweeper_thread->set_entry_point(&WhiteBox::sweeper_thread_entry); 851 852 // check that thread and osthread were created 853 if (sweeper_thread == NULL || sweeper_thread->osthread() == NULL) { 854 vm_exit_during_initialization("java.lang.OutOfMemoryError", 855 os::native_thread_creation_failed_msg());

This should be vm_exit_out_of_memory, not "during initialization".

856 } 857

/Mikael

Problem: The test uses the Whitebox API to enforce sweeping by creating and starting a 'CodeCacheSweeperThread'. During creation of the thread, the interpreter crashes in j.l.ThreadGroup.add(Thread t) [1] while executing a subtype check to validate that 't' is a subtype of j.l.Thread [2]. The problem is that we pass 'JavaThread->threadObj()' to 'ThreadGroup.add' which is invalid due to a GC that moved the object. The GC does not know about the thread because it was not yet added to the threads list and therefore does not update the oop. Solution: Instead of calling 'JavaThread::allocatethreadObj', the initialization is moved to the caller to make sure that setting the thread oop is done together with adding the thread to the threads list. I also fixed the missing oom handling described as one of the problems in JDK-8072377 [3]. Testing: - 1k runs of failing testcase - JPRT Thanks, Tobias [1] http://hg.openjdk.java.net/jdk9/hs-comp/jdk/file/tip/src/java.base/share/classes/java/lang/ThreadGroup.java#l896 [2] see ' gensubtypecheck' in 'TemplateTable::aastore'_ [3] https://bugs.openjdk.java.net/browse/JDK-8072377

More information about the hotspot-compiler-dev mailing list