RFR: 8259486: Remove CautiouslyPreserveExceptionMark

Coleen Phillimore coleenp at openjdk.java.net
Mon Jan 11 12:58:02 UTC 2021


On Mon, 11 Jan 2021 01:13:25 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> See CR for details.  This is a hopefully simple cleanup.  I didn't try to reason about removing any of these except an obvious call (if you look above the patch in thread.cpp).
>> Tested with tier1-3.
>
> src/hotspot/share/runtime/thread.cpp line 1867:
> 
>> 1865:       // Call method Thread.dispatchUncaughtException().
>> 1866:       Klass* thread_klass = SystemDictionary::Thread_klass();
>> 1867:       JavaValue result(T_VOID);
> 
> I think I agree that this is redundant. Seems to be a leftover from something else.

Yes.  Look above it.  It clears the exception mark so there's never an exception pending.  strange we didn't clean this out sometime between 1.2 and now.

> src/hotspot/share/classfile/javaClasses.cpp line 2528:
> 
>> 2526:         if (total_count == 0) {
>> 2527:           // The top frame will be hidden from the stack trace.
>> 2528:           bt.set_has_hidden_top_frame();
> 
> To be clear: no exception can become pending due to this call, so no need for CHECK.

Correct.  I think it used to allocate an object so could throw OOM but that was changed.

> src/hotspot/share/classfile/javaClasses.cpp line 2556:
> 
>> 2554: 
>> 2555:   JavaThread* thread = JavaThread::active();
>> 2556:   PreserveExceptionMark pm(thread);
> 
> The result of JavaThread::active() is not necessarily the current thread. So either the new code is wrong or else we should just be using JavaThread::current()

My first iteration of this change knew there was a difference then I cleaned it up.  I'll restore the Thread::current() in the PEM constructor.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2007


More information about the serviceability-dev mailing list