# HG changeset patch # User mdoerr # Date 1574097678 -3600 # Node ID 5ac4a49f539939f0dc659a25cbc256eb6d801392 # Parent d01fe40e9cd8a2153dae3a63d21b297c07681dd8 8233193: Incorrect bailout from possibly_add_compiler_threads Reviewed-by: dholmes, thartmann diff -r d01fe40e9cd8 -r 5ac4a49f5399 src/hotspot/share/compiler/compileBroker.cpp --- a/src/hotspot/share/compiler/compileBroker.cpp Mon Nov 18 16:48:05 2019 +0000 +++ b/src/hotspot/share/compiler/compileBroker.cpp Mon Nov 18 18:21:18 2019 +0100 @@ -596,7 +596,7 @@ // CompileBroker::compilation_init // // Initialize the Compilation object -void CompileBroker::compilation_init_phase1(TRAPS) { +void CompileBroker::compilation_init_phase1(Thread* THREAD) { // No need to initialize compilation system if we do not use it. if (!UseCompiler) { return; @@ -647,6 +647,7 @@ // totalTime performance counter is always created as it is required // by the implementation of java.lang.management.CompilationMBean. { + // Ensure OOM leads to vm_exit_during_initialization. EXCEPTION_MARK; _perf_total_compilation = PerfDataManager::create_counter(JAVA_CI, "totalTime", @@ -761,17 +762,17 @@ } -JavaThread* CompileBroker::make_thread(jobject thread_handle, CompileQueue* queue, AbstractCompiler* comp, TRAPS) { - JavaThread* thread = NULL; +JavaThread* CompileBroker::make_thread(jobject thread_handle, CompileQueue* queue, AbstractCompiler* comp, Thread* THREAD) { + JavaThread* new_thread = NULL; { MutexLocker mu(Threads_lock, THREAD); if (comp != NULL) { if (!InjectCompilerCreationFailure || comp->num_compiler_threads() == 0) { CompilerCounters* counters = new CompilerCounters(); - thread = new CompilerThread(queue, counters); + new_thread = new CompilerThread(queue, counters); } } else { - thread = new CodeCacheSweeperThread(); + new_thread = new CodeCacheSweeperThread(); } // At this point the new CompilerThread data-races with this startup // thread (which I believe is the primoridal thread and NOT the VM @@ -786,9 +787,9 @@ // exceptions anyway, check and abort if this fails. But first release the // lock. - if (thread != NULL && thread->osthread() != NULL) { + if (new_thread != NULL && new_thread->osthread() != NULL) { - java_lang_Thread::set_thread(JNIHandles::resolve_non_null(thread_handle), thread); + java_lang_Thread::set_thread(JNIHandles::resolve_non_null(thread_handle), new_thread); // Note that this only sets the JavaThread _priority field, which by // definition is limited to Java priorities and not OS priorities. @@ -809,24 +810,24 @@ native_prio = os::java_to_os_priority[NearMaxPriority]; } } - os::set_native_priority(thread, native_prio); + os::set_native_priority(new_thread, native_prio); java_lang_Thread::set_daemon(JNIHandles::resolve_non_null(thread_handle)); - thread->set_threadObj(JNIHandles::resolve_non_null(thread_handle)); + new_thread->set_threadObj(JNIHandles::resolve_non_null(thread_handle)); if (comp != NULL) { - thread->as_CompilerThread()->set_compiler(comp); + new_thread->as_CompilerThread()->set_compiler(comp); } - Threads::add(thread); - Thread::start(thread); + Threads::add(new_thread); + Thread::start(new_thread); } } // First release lock before aborting VM. - if (thread == NULL || thread->osthread() == NULL) { + if (new_thread == NULL || new_thread->osthread() == NULL) { if (UseDynamicNumberOfCompilerThreads && comp != NULL && comp->num_compiler_threads() > 0) { - if (thread != NULL) { - thread->smr_delete(); + if (new_thread != NULL) { + new_thread->smr_delete(); } return NULL; } @@ -837,11 +838,12 @@ // Let go of Threads_lock before yielding os::naked_yield(); // make sure that the compiler thread is started early (especially helpful on SOLARIS) - return thread; + return new_thread; } void CompileBroker::init_compiler_sweeper_threads() { + // Ensure any exceptions lead to vm_exit_during_initialization. EXCEPTION_MARK; #if !defined(ZERO) assert(_c2_count > 0 || _c1_count > 0, "No compilers?"); @@ -875,7 +877,7 @@ _compiler2_logs[i] = NULL; if (!UseDynamicNumberOfCompilerThreads || i == 0) { - JavaThread *ct = make_thread(thread_handle, _c2_compile_queue, _compilers[1], CHECK); + JavaThread *ct = make_thread(thread_handle, _c2_compile_queue, _compilers[1], THREAD); assert(ct != NULL, "should have been handled for initial thread"); _compilers[1]->set_num_compiler_threads(i + 1); if (TraceCompilerThreads) { @@ -895,7 +897,7 @@ _compiler1_logs[i] = NULL; if (!UseDynamicNumberOfCompilerThreads || i == 0) { - JavaThread *ct = make_thread(thread_handle, _c1_compile_queue, _compilers[0], CHECK); + JavaThread *ct = make_thread(thread_handle, _c1_compile_queue, _compilers[0], THREAD); assert(ct != NULL, "should have been handled for initial thread"); _compilers[0]->set_num_compiler_threads(i + 1); if (TraceCompilerThreads) { @@ -914,12 +916,11 @@ // Initialize the sweeper thread Handle thread_oop = create_thread_oop("Sweeper thread", CHECK); jobject thread_handle = JNIHandles::make_local(THREAD, thread_oop()); - make_thread(thread_handle, NULL, NULL, CHECK); + make_thread(thread_handle, NULL, NULL, THREAD); } } -void CompileBroker::possibly_add_compiler_threads() { - EXCEPTION_MARK; +void CompileBroker::possibly_add_compiler_threads(Thread* THREAD) { julong available_memory = os::available_memory(); // If SegmentedCodeCache is off, both values refer to the single heap (with type CodeBlobType::All). @@ -970,7 +971,7 @@ _compiler2_objects[i] = thread_handle; } #endif - JavaThread *ct = make_thread(compiler2_object(i), _c2_compile_queue, _compilers[1], CHECK); + JavaThread *ct = make_thread(compiler2_object(i), _c2_compile_queue, _compilers[1], THREAD); if (ct == NULL) break; _compilers[1]->set_num_compiler_threads(i + 1); if (TraceCompilerThreads) { @@ -990,7 +991,7 @@ (int)(available_cc_p / (128*K))); for (int i = old_c1_count; i < new_c1_count; i++) { - JavaThread *ct = make_thread(compiler1_object(i), _c1_compile_queue, _compilers[0], CHECK); + JavaThread *ct = make_thread(compiler1_object(i), _c1_compile_queue, _compilers[0], THREAD); if (ct == NULL) break; _compilers[0]->set_num_compiler_threads(i + 1); if (TraceCompilerThreads) { @@ -1511,14 +1512,6 @@ } // ------------------------------------------------------------------ -// CompileBroker::preload_classes -void CompileBroker::preload_classes(const methodHandle& method, TRAPS) { - // Move this code over from c1_Compiler.cpp - ShouldNotReachHere(); -} - - -// ------------------------------------------------------------------ // CompileBroker::create_compile_task // // Create a CompileTask object representing the current request for @@ -1865,7 +1858,8 @@ } if (UseDynamicNumberOfCompilerThreads) { - possibly_add_compiler_threads(); + possibly_add_compiler_threads(thread); + assert(!thread->has_pending_exception(), "should have been handled"); } } } diff -r d01fe40e9cd8 -r 5ac4a49f5399 src/hotspot/share/compiler/compileBroker.hpp --- a/src/hotspot/share/compiler/compileBroker.hpp Mon Nov 18 16:48:05 2019 +0000 +++ b/src/hotspot/share/compiler/compileBroker.hpp Mon Nov 18 18:21:18 2019 +0100 @@ -226,11 +226,10 @@ static volatile int _print_compilation_warning; static Handle create_thread_oop(const char* name, TRAPS); - static JavaThread* make_thread(jobject thread_oop, CompileQueue* queue, AbstractCompiler* comp, TRAPS); + static JavaThread* make_thread(jobject thread_oop, CompileQueue* queue, AbstractCompiler* comp, Thread* THREAD); static void init_compiler_sweeper_threads(); - static void possibly_add_compiler_threads(); + static void possibly_add_compiler_threads(Thread* THREAD); static bool compilation_is_prohibited(const methodHandle& method, int osr_bci, int comp_level, bool excluded); - static void preload_classes (const methodHandle& method, TRAPS); static CompileTask* create_compile_task(CompileQueue* queue, int compile_id, @@ -292,7 +291,7 @@ CompileQueue *q = compile_queue(comp_level); return q != NULL ? q->size() : 0; } - static void compilation_init_phase1(TRAPS); + static void compilation_init_phase1(Thread* THREAD); static void compilation_init_phase2(); static void init_compiler_thread_log(); static nmethod* compile_method(const methodHandle& method,