RFR: 8259486: Remove CautiouslyPreserveExceptionMark

[David Holmes]

On Fri, 8 Jan 2021 16:58:15 GMT, Coleen Phillimore <coleenp 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.

Hi Coleen,

AFAICS you are not simply removing CautiouslyPreserveExceptionMark but you are changing PreserveExceptionMark to now act as CautiouslyPreserveExceptionMark  did. If the PEM now encounters an unexpected exception it is no longer fatal. This relaxation is not unreasonable but I think the bug synopsis and description need to be updated to reflect what you are actually doing.

I agree "Cautiously" was not a good word for distinguishing the two responses here. I also think WeakPEM is not great either. :(

A couple of minor comments elsewhere.

Thanks,
David

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.

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()

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.

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

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