RFR(S): 7037756 Deadlock in compiler thread similiar to 6789220

[Tom Rodriguez]

On Apr 25, 2011, at 7:20 PM, Y. Srinivas Ramakrishna wrote:

> On 4/25/2011 5:12 PM, Tom Rodriguez wrote:
>> Seems ok.  We'll never compile the main loop of the reference handler thread new but that doesn't seem like it should matter much.
> 
> May be test with a benchmark that stresses Reference object handling.
> May be the bottleneck will still not be the reference handler thread
> running interpreted code, but worth checking (may be later).
> 
> Can the compiler thread be prevented from trying to allocate
> in the heap while holding the Compile_lock? (Would it work
> for it to drop and reacquire the lock around such allocations,
> or is that not feasible? Just doing some loud thinking.)

Well the compiler itself isn't acquiring the Compile_lock.  It appears to be this code in objArrayKlassKlass.

        KlassHandle ek;
        {
          MutexUnlocker mu(MultiArray_lock);
          MutexUnlocker mc(Compile_lock);   // for vtables                                                                                  
          klassOop sk = element_super->array_klass(CHECK_0);
          super_klass = KlassHandle(THREAD, sk);
          for( int i = element_supers->length()-1; i >= 0; i-- ) {
            KlassHandle elem_super (THREAD, element_supers->obj_at(i));
            elem_super->array_klass(CHECK_0);
          }
          // Now retry from the beginning                                                                                                    
          klassOop klass_oop = element_klass->array_klass(n, CHECK_0);
          // Create a handle because the enclosing brace, when locking                                                                      
          // can cause a gc.  Better to have this function return a Handle.                                                                  
          ek = KlassHandle(THREAD, klass_oop);
        }  // re-lock                                                                                                                        
        return ek();

I really don't understand the usage of Compile_lock here or the comment "for vtables".  All the array klasses do something similar and I don't get it.  The main purpose of Compile_lock is control updates to the system dictionary but I don't see why the array klasses have to do anything special here.

> 
> (Or could one safely pre-compile the reference handler's code
> before start-up so it doesn't always run interpreted?)

We could have a small side queue that we could enqueue these on and when a later safe request comes in we can request a non blocking compile of them.  It's kind of gross but it would work I think.  It could even just be a queue of length one instead of a full data structure.

tom

> 
> -- ramki
> 
>> 
>> tom
>> 
>> On Apr 25, 2011, at 4:37 PM, John Cuthbertson wrote:
>> 
>>> Hi Everyone,
>>> 
>>> A new webrev that is essentially Tom's suggestion can be found at: http://cr.openjdk.java.net/~johnc/7037756/webrev.2/. I also reverted Bengt's fix for 6789220 as, with Tom's suggested fix, a thread that owns the pending list will no longer be blocked in CompileBroker::compile_method_base.
>>> 
>>> Testing: Ran over the weekend with the test case for 7037756; the test case for 6789220 (which fails 50% of the time with Bengt's fix removed); nsk tests; jprt.
>>> 
>>> Thanks,
>>> 
>>> JohnC
>>> 
>>> 
>>> On 04/22/11 18:48, Tom Rodriguez wrote:
>>>> Instead of enshrining the reference handler thread itself, could you make it work by checking whether the requesting thread owns the reference handler lock instead?  That seems more robust and targeted.   Something like:
>>>> 
>>>> if (instanceRefKlass:owns_pending_list_lock(JavaThread::current()) {
>>>>   return false;
>>>> }
>>>> 
>>>> replacing the fix in in CompileBroker::is_compile_blocking seems like it should work.
>>>> 
>>>> tom
>>>> 
>>>> On Apr 22, 2011, at 5:38 PM, John Cuthbertson wrote:
>>>> 
>>>> 
>>>> 
>>>>> Hi EVeryone.
>>>>> 
>>>>> Typo....
>>>>> 
>>>>> On 04/22/11 17:28, John Cuthbertson wrote:
>>>>> 
>>>>> 
>>>>>> Hi Everyone,
>>>>>> 
>>>>>> Can I have a couple of volunteers to look over these changes? The webrev can be found at:
>>>>>> http://cr.openjdk.java.net/~johnc/7037756/webrev.1
>>>>>> 
>>>>>> 
>>>>>> The issue here was very similar to the issue that caused 6789220 - the difference here was that the reference handler was blocked while waiting for the MethodCompileQueue_lock rather than waiting on a blocking compilation. To summarize:
>>>>>> 
>>>>>> Thread 6 (reference handler thread), while owning the pending list lock, requested a compilation and was blocked waiting on the MethodCompileQueue_lock.
>>>>>> 
>>>>>> Thread 11 (compiler thread 1), while owning the Compile_lock, attempted to allocate a Class mirror which triggered GC. In the GC it was blocked attempting to lock the pending list lock.
>>>>>> 
>>>>>> Thread 12 (compiler thread 2) was registering a compiled method and, while owning the MethodCompileQueue_lock, was blocked waiting on the Compile_lock.
>>>>>> 
>>>>>> The solution is to make the reference handler thread not block while holding the pending list lock. If the requesting thread is the reference handler thread, then an attempt is made to lock the MethodCompileQueue_lock in CompileBroker::compile_method_base and, if that is unsuccessful, we just return with enqueueing the compile task. Otherwise a regular blocking lock attempt is made. I also tweaked the fix made by Bengt for 6789220 to make all compilation requests by the reference handler thread non-blocking.
>>>>>> 
>>>>>> 
>>>>> The above paragraph should read;
>>>>> 
>>>>> The solution is to make the reference handler thread not block while holding the pending list lock. If the requesting thread is the reference handler thread, then an attempt is made to lock the MethodCompileQueue_lock in CompileBroker::compile_method_base and, if that is unsuccessful, we just return _without_ enqueueing the compilation request. Otherwise a regular blocking lock attempt is made. I also tweaked the fix made by Bengt for 6789220 to make all compilation requests by the reference handler thread non-blocking.
>>>>> 
>>>>> 
>>>>>> Testing: the failing test case has been running successfully on the VMSQE machine for 2 days (normally I see the deadlock after 20 minutes or so); the nsk tests; and a jprt job is the queue.
>>>>>> 
>>>>>> Thanks,
>>>>>> 
>>>>>> JohnC
>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> 
>>>> 
>>> 
>> 
>