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

David Holmes david.holmes at oracle.com
Thu Feb 20 10:40:48 UTC 2014

Hi Jaroslav,


Comment is wrong:

913     // JVMTI internal flag reset is needed in order to report 

It will be ExceptionInInitializerError

You added this:

this_oop->set_initialization_state_and_notify(initialization_error, THREAD);
   918       CLEAR_PENDING_EXCEPTION;   // ignore any exception thrown, 
class initialization error is thrown below
+ 919       // JVMTI has already reported the pending exception
+ 920       // JVMTI internal flag reset is needed in order to report 
+ 921       JvmtiExport::clear_detected_exception((JavaThread*)THREAD);

but there are a number of places where 
set_initialization_state_and_notify is called when a pending exception 
has been cleared, and then CLEAR_PENDING_EXCEPTION is called again, but 
you didn't modify those other locations. They will rethrow the original 
exception so I suppose that is okay from JVMTI's perspective. But the 
flip-side of this is that if set_initialization_state_and_notify does 
throw an exception, JVMTI will never see it.



Comment is wrong again - not InvocationTargetException.



On 20/02/2014 1:59 AM, Jaroslav Bachorik wrote:
> 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:
> http://prt-web.us.oracle.com//archive/2014/02/2014-02-19-114618.jbachorik.hotspot/
> Aurora Adhoc:
> http://aurora.ru.oracle.com//faces/Batch.xhtml?batchName=418853.VMSQE.adhoc.JPRT.full
> (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