RFR 4505697: nsk/jdi/ExceptionEvent/_itself_/exevent006 and exevent008 tests fail with InvocationTargetException

Jaroslav Bachorik jaroslav.bachorik at oracle.com
Wed Feb 19 15:59:08 UTC 2014

On 18.2.2014 11:18, serguei.spitsyn at oracle.com wrote:
> On 2/17/14 12:04 AM, Jaroslav Bachorik wrote:
>> On 14.2.2014 23:13, serguei.spitsyn at oracle.com wrote:
>>> On 2/14/14 12:33 PM, Daniel D. Daugherty wrote:
>>>> On 2/14/14 11:46 AM, serguei.spitsyn at oracle.com wrote:
>>>>> Jaroslav,
>>>>> It looks good in general modulo indent comments from Dan.
>>>>> But I have a doubt that acquiring the JvmtiThreadState_lock is needed
>>>>> or right thing to do in the JvmtiExport::clear_detected_exception().
>>>>> It seems, both clear_exception_detected() and
>>>>> set_exception_detected() are always
>>>>> called on current thread and so, it has to be safe to do without
>>>>> acquiring any locks.
>>>> My JVM/TI-foo is rusty, but I believe that JvmtiThreadState stuff
>>>> can also be queried/modified by other threads so grabbing the
>>>> associated lock is a good idea.
>>> The lock synchronization is cooperative.
>>> It does not help much if the lock is not acquired in other places.
>>> I can be wrong, but I've not found yet any place in the code where the
>>> clear_exception_detected() and set_exception_detected() are called
>>> under protection of the JvmtiThreadState_lock.
>> I copied the locking over from
>> "JvmtiExport::cleanup_thread(JavaThread* thread)". That method is also
>> supposed to work only with the current thread but acquires the lock
>> nonetheless. But if you are sure that the lock is not required I have
>> no objections removing it.
> I'm suggesting to remove it, as it is not used in other places in the code.
> It is going to be confusing if it is used in one place and missed in
> others.

I've removed the lock and applied the same cleanup logic to other places 
where exceptions are rewrapped.

Webrev: http://cr.openjdk.java.net/~jbachorik/4505697/webrev.02
JPRT run: 
Aurora Adhoc: 
(still running at the moment; no failures so far)

> Thanks,
> Serguei
>> -JB-
>>> Thanks,
>>> Serguei
>>>> Dan
>>>>> And I'm repeating my question about pre-integration testing (Dan is
>>>>> asking about the same).
>>>>> Thanks,
>>>>> Serguei
>>>>> On 2/14/14 3:07 AM, Jaroslav Bachorik wrote:
>>>>>> This is a round-0 review request.
>>>>>> The reflection code intercepting the exceptions thrown in the
>>>>>> invoked methods does not play nicely with JVMTI (which, in this
>>>>>> case, propagates to JDI).
>>>>>> The reflection code lacks the traditional error handler - therefore,
>>>>>> upon throwing the NumberFormatException, the stack is searched for
>>>>>> appropriate handlers and none are found. This leaves the
>>>>>> "exception_detected" flag set to true while normally it would be
>>>>>> reset to false once the exception is handled. The reflection code
>>>>>> then goes on and wraps the NumberFormatException into
>>>>>> InvocationTargetException and throws it. But, alas, the
>>>>>> "exception_detected" flag is still set to true and no JVMTI
>>>>>> exception event will be sent out.
>>>>>> The proposed solution is to call
>>>>>> thread->jvmti_thread_state()->clear_exception_detected() at the
>>>>>> appropriate places in the reflection code to reset the
>>>>>> "exception_detected" flag and enable the InvocationTargetException
>>>>>> be properly reported over JVMTI.
>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-4505697
>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/4505697/webrev.00
>>>>>> Thanks!
>>>>>> -JB-

More information about the hotspot-runtime-dev mailing list