RFR(M): JDK-8165496 assert(_exception_caught == false) failed: _exception_caught is out of phase

[Dmitry Samersoff]

Serguei.

I intentionally decide not to provide generic accessor functions
for _exception_state.

Developers should use set_exception_detected(),  set_exception_caught(),
 clear_exception_state() but don't change _exception_state directly.

And save_exception_state()/restore_exception_state() name and signature
clear indicates that the only valid usage of these functions is backup
of exception_state.

So I would prefer to leave it as is.

-Dmitry


On 2016-12-06 08:57, serguei.spitsyn at oracle.com wrote:
> Hi Dmitry,
> 
> 
> I'm repeating my comments from the internal review.
> 
> It looks pretty good to me.
> Thank you for the work on it!
> 
> I'm not a big fun of new jvmtiThreadState functions though:
> 
> + inline void save_exception_state(ExceptionState *state) { *state =
> _exception_state; }
> + inline void restore_exception_state(ExceptionState state) {
> _exception_state = state; } The functions above do not really
> encapsulate and save/restore the _exception_state value so that these
> names are kind of misleading. The same functionality can be provided
> with a simplified approach:     inline JvmtiThreadState
> exception_state() { return_exception_state;  }
>    inline void set_exception_state(ExceptionState state){
> _exception_state = state; }
> 
>  
> I do not want to insist on my approach and will wait for other people
> comments on this.
> I hope, some compromise can be found here.
> 
> 
> On 12/5/16 01:30, Dmitry Samersoff wrote:
>> Everybody,
>>
>> Please review.
>>
>> http://cr.openjdk.java.net/~dsamersoff/JDK-8165496/webrev.07/
>>
>> Bug link:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8165496
>>
>> Two separate flags, exception_detected and exception_caught, replaced
>> with one that changes it's state.
>>
>> Assert assert(_exception_caught == false) and fix for JDK-8134434 are
>> removed.
>>
>> Testing:
>>    1. Manual testing of instrumented hotspot
>>    2. Local ks with stress agent
>>    3. RBT ks with stress agent
> 
> What the 'ks' stands for?
> I have some guess but still would like to know this abbreviation.
> 
> Thanks,
> Serguei
> 
>>    4. RBT hotspot_all
>>
>> -Dmitry
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.