RFR[13]: 8227260: Can't deal with SharedRuntime::handle_wrong_method triggering more than once for interpreter calls

[Dmitrij Pochepko]

Hi,

looks fine.

I also ran updated ClassInitBarrier test on aarch64 fastdebug build

Thanks,

Dmitrij

On 17/07/2019 4:06 PM, Vladimir Ivanov wrote:
> Thanks, Erik.
>
> Also, since I touch platform-specific code, I'd like Martin and 
> Dmitrij (implementors of support for s390, ppc, and aarch64) to take a 
> look at the patch as well.
>
> Best regards,
> Vladimir Ivanov
>
> On 17/07/2019 15:25, Erik Österlund wrote:
>> Hi Vladimir,
>>
>> Looks good. Thanks for fixing.
>>
>> /Erik
>>
>> On 2019-07-17 12:26, Vladimir Ivanov wrote:
>>> Revised fix:
>>>    http://cr.openjdk.java.net/~vlivanov/8227260/webrev.00/
>>>
>>> It turned out the problem is not specific to i2c2i: fast class 
>>> initialization barriers on nmethod entry trigger the assert as well.
>>>
>>> JNI upcalls (CallStatic<type>Method) don't have class initialization 
>>> checks, so it's possible to initiate a JNI upcall from a 
>>> non-initializing thread and JVM should let it complete.
>>>
>>> It leads to a busy loop (asserts in debug) between nmethod entry 
>>> barrier & SharedRuntime::handle_wrong_method until holder class is 
>>> initialized (possibly infinite if it blocks class initialization).
>>>
>>> Proposed fix is to keep using c2i, but jump over class 
>>> initialization barrier right to the argument shuffling logic on 
>>> verified entry when coming from SharedRuntime::handle_wrong_method.
>>>
>>> Improved regression test reliably reproduces the problem.
>>>
>>> Testing: regression test, hs-precheckin-comp, tier1-6
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 04/07/2019 18:02, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> The i2c adapter sets a thread-local "callee_target" Method*, which 
>>>> is caught (and cleared) by SharedRuntime::handle_wrong_method if 
>>>> the i2c call is "bad" (e.g. not_entrant). This error handler 
>>>> forwards execution to the callee c2i entry. If the 
>>>> SharedRuntime::handle_wrong_method method is called again due to 
>>>> the i2c2i call being still bad, then we will crash the VM in the 
>>>> following guarantee in SharedRuntime::handle_wrong_method:
>>>>
>>>> Method* callee = thread->callee_target();
>>>> guarantee(callee != NULL && callee->is_method(), "bad handshake");
>>>>
>>>> Unfortunately, the c2i entry can indeed fail again if it, e.g., 
>>>> hits the new class initialization entry barrier of the c2i adapter.
>>>> The solution is to simply not clear the thread-local 
>>>> "callee_target" after handling the first failure, as we can't 
>>>> really know there won't be another one. There is no reason to clear 
>>>> this value as nobody else reads it than the 
>>>> SharedRuntime::handle_wrong_method handler (and we really do want 
>>>> it to be able to read the value as many times as it takes until the 
>>>> call goes through). I found some confused clearing of this 
>>>> callee_target in JavaThread::oops_do(), with a comment saying this 
>>>> is a methodOop that we need to clear to make GC happy or something. 
>>>> Seems like old traces of perm gen. So I deleted that too.
>>>>
>>>> I caught this in ZGC where the timing window for hitting this issue 
>>>> seems to be wider due to concurrent code cache unloading. But it is 
>>>> equally problematic for all GCs.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8227260
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8227260/webrev.00/
>>>>
>>>> Thanks,
>>>> /Erik