JDK-6989981: jstack causes "fatal error: ExceptionMark destructor expects no pending exceptions"
David Holmes
david.holmes at oracle.com
Thu Oct 3 17:14:28 PDT 2013
On 2/10/2013 12:11 AM, Yasumasa Suenaga wrote:
> Hi David,
>
>> Other than printing a message on the console it pretends that the init
>> has succeeded!
>
> No.
>
> signal_thread_entry() @ hotspot/src/share/vm/runtime/os.cpp
> --------
> 250 case SIGBREAK: {
> 251 // Check if the signal is a trigger to start the
> Attach Listener - in that
> 252 // case don't print stack traces.
> 253 if (!DisableAttachMechanism &&
> AttachListener::is_init_trigger()) {
> 254 continue;
> 255 }
> --------
>
> AttachListener::init() is called through is_init_trigger() (line 505).
>
> AttachListener::is_init_trigger() @
> hotspot/src/share/vm/services/attachListener.cpp
> --------
> 501 if (ret == 0) {
> 502 // simple check to avoid starting the attach mechanism when
> 503 // a bogus user creates the file
> 504 if (st.st_uid == geteuid()) {
> 505 init();
> 506 return true;
> 507 }
> 508 }
> --------
>
> If exception occurs in AttachListner::init(),
> AttachListner::is_init_trigger() returns "true" .
> Process of SIGBREAK handler will be stopped (caused by continue
> statement) and signal_thread_entry()
> will return top of loop.
>
> In my patch, pending exception which occurs in AttachListener
> initialization is cleared.
> Thus Signal Dispatcher has no damage.
But as I said the init() fails and we pretend it succeeded with respect
to the caller. Now for the signal case if the init fails we can simply
do nothing - the VM is running, this particular action failed, so the VM
continues - that's okay. So I'm okay with the fix in that regard.
In general I think AttachListener::init should return a boolean to
indicate success or failure so that the caller can decide what needs to
be done eg at VM startup it seems reasonable to call
vm_exit_during_initialization because we can't pre-start the listener as
we are required to do.
Though as Staffan pointed out init() incorrectly calls
vm_exit_during_initialization in other places. :(
>> What are the consequences of doing this?
>
> If AttachListener::init() cannot work due to exception, it cannot create
> UNIX domain socket to accept from java tools (e.g. jstack) .
> Target VM which tried to invoke AttachListner continues to run.
> (However, runtime exception such as OOME occurs, target VM will be
> stopped immediately :-)
>
> On the other hand, java tools which tried to attach target VM will fail
> because it cannot connect UNIX domain socket.
Will the tool fail differently now compared to the VM aborting? Purely
from a testing perspective we tend to notice VM aborts but the console
print about the exception may well go unnoticed.
Thanks,
David
>
> Please run my testcase. It's actual behavior.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2013/10/01 22:15, David Holmes wrote:
>> On 23/09/2013 2:41 AM, Staffan Larsen wrote:
>>> Dmitry: Thanks for the review.
>>>
>>> Yasumasa: Thanks for your contribution! I have pushed the changes
>>> into HS25 and will push them to 7u60 as well.
>>
>> I've been on vacation so could not respond in time. I am concerned
>> about this fix. Other than printing a message on the console it
>> pretends that the init has succeeded! That seems wrong to me. What are
>> the consequences of doing this?
>>
>> David
>> -----
>>
>>> /Staffan
>>>
>>> On 22 sep 2013, at 01:51, Dmitry Samersoff
>>> <Dmitry.Samersoff at oracle.com> wrote:
>>>
>>>> Yasumasa,
>>>>
>>>> Looks good for me.
>>>>
>>>> -Dmitry
>>>>
>>>> On 2013-09-21 14:43, Yasumasa Suenaga wrote:
>>>>> Hi Staffan,
>>>>>
>>>>>> Can you update your patch, please?
>>>>>
>>>>> Of course!
>>>>> I've attached new patch in this email.
>>>>>
>>>>> Could you check this?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>> On 2013/09/21 15:36, Staffan Larsen wrote:
>>>>>>
>>>>>>
>>>>>>> On 20 sep 2013, at 16:49, Yasumasa Suenaga<yasu at ysfactory.dip.jp>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On 2013/09/20 23:34, Staffan Larsen wrote:
>>>>>>>>
>>>>>>>>> On 20 sep 2013, at 16:24, Yasumasa Suenaga<yasu at ysfactory.dip.jp>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> I thought your code too. However...
>>>>>>>>>
>>>>>>>>> - These code is different from other code (rule?).
>>>>>>>>
>>>>>>>> Well, you are introducing a new macro that is also different from
>>>>>>>> other code, so I'm not sure how valid that argument is.
>>>>>>>
>>>>>>> My macro is modified from "CATCH" in exceptions.hpp:
>>>>>>>
>>>>>>> #define CATCH \
>>>>>>> THREAD); if (HAS_PENDING_EXCEPTION) { \
>>>>>>> oop ex = PENDING_EXCEPTION; \
>>>>>>> CLEAR_PENDING_EXCEPTION; \
>>>>>>> ex->print(); \
>>>>>>> ShouldNotReachHere(); \
>>>>>>> } (void)(0
>>>>>>>
>>>>>>> So I think that my macro is not big difference fromother code.
>>>>>>>
>>>>>>>
>>>>>>>>> - Similar crash cases exist. e.g. 6425580 and 7142442.
>>>>>>>>> These crashes are different from 6989981. However, I guess that
>>>>>>>>> crashed
>>>>>>>>> thread had pending exception and we need to fix with similar
>>>>>>>>> patch.
>>>>>>>>>
>>>>>>>>> So I think that new macro is useful later.
>>>>>>>>
>>>>>>>> Yes, similar problems may come up in other cases as well.
>>>>>>>>
>>>>>>>> Generally, I don't think it's a good idea to have logging calls
>>>>>>>> hidden away in general macros. What we really should do here is
>>>>>>>> print some context around the stack trace as well. Something like:
>>>>>>>>
>>>>>>>> Initializing the attach listener failed with the following
>>>>>>>> exception in AttachListener::init when initializing the thread_oop:
>>>>>>>>
>>>>>>>> This would be possible with the code I suggested, but very hard
>>>>>>>> in a
>>>>>>>> general macro.
>>>>>>>
>>>>>>> Agree.
>>>>>>> Should we write code as following?
>>>>>>>
>>>>>>> if (HAS_PENDING_EXCEPTION) {
>>>>>>> tty->print_cr("Exception in VM (AttachListener::init) : ");
>>>>>>> java_lang_Throwable::print(PENDING_EXCEPTION, tty);
>>>>>>> CLEAR_PENDING_EXCEPTION;
>>>>>>> return;
>>>>>>> }
>>>>>>>
>>>>>>> I like this way :-)
>>>>>>
>>>>>> Yes, this is what I'd like to see! Can you update your patch, please?
>>>>>>
>>>>>> Thanks,
>>>>>> /Staffan
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Yasumasa
>>>>>>>
>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> /Staffan
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Yasumasa
>>>>>>>>>
>>>>>>>>>> On 2013/09/20 23:05, Staffan Larsen wrote:
>>>>>>>>>> I see. CHECK_AND_CLEAR_AND_PRINT? Just kidding... :-)
>>>>>>>>>>
>>>>>>>>>> Maybe in this case we should not have this as a macro, but
>>>>>>>>>> actually add the code after the two calls to call_special?
>>>>>>>>>> Something like the code below. I personally think this is more
>>>>>>>>>> readable than obscure macros that I have to go look up to
>>>>>>>>>> understand what they do.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> /Staffan
>>>>>>>>>>
>>>>>>>>>> JavaCalls::call_special(&result, thread_oop,
>>>>>>>>>> klass,
>>>>>>>>>> vmSymbols::object_initializer_name(),
>>>>>>>>>>
>>>>>>>>>> vmSymbols::threadgroup_string_void_signature(),
>>>>>>>>>> thread_group,
>>>>>>>>>> string,
>>>>>>>>>> THREAD);
>>>>>>>>>>
>>>>>>>>>> if (HAS_PENDING_EXCEPTION) {
>>>>>>>>>> java_lang_Throwable::print(PENDING_EXCEPTION, tty);
>>>>>>>>>> CLEAR_PENDING_EXCEPTION;
>>>>>>>>>> return;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> KlassHandle group(THREAD, SystemDictionary::ThreadGroup_klass());
>>>>>>>>>> JavaCalls::call_special(&result,
>>>>>>>>>> thread_group,
>>>>>>>>>> group,
>>>>>>>>>> vmSymbols::add_method_name(),
>>>>>>>>>> vmSymbols::thread_void_signature(),
>>>>>>>>>> thread_oop, // ARG 1
>>>>>>>>>> THREAD);
>>>>>>>>>>
>>>>>>>>>> if (HAS_PENDING_EXCEPTION) {
>>>>>>>>>> java_lang_Throwable::print(PENDING_EXCEPTION, tty);
>>>>>>>>>> CLEAR_PENDING_EXCEPTION;
>>>>>>>>>> return;
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On 20 sep 2013, at 15:53, Yasumasa
>>>>>>>>>>> Suenaga<yasu at ysfactory.dip.jp> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Staffan,
>>>>>>>>>>>
>>>>>>>>>>> Thank you for your sponsoring!
>>>>>>>>>>>
>>>>>>>>>>> "CHECK_AND_CLEAR" is already defined in exceptions.hpp:
>>>>>>>>>>> ******************
>>>>>>>>>>> #define CHECK_AND_CLEAR THREAD); if
>>>>>>>>>>> (HAS_PENDING_EXCEPTION) { CLEAR_PENDING_EXCEPTION; return;
>>>>>>>>>>> } (void)(0
>>>>>>>>>>> ******************
>>>>>>>>>>>
>>>>>>>>>>> I think that user wants why serviceability tools are failed.
>>>>>>>>>>> So I defined "CHECK_AND_CLEAR" + java_lang_Throwable::print() as
>>>>>>>>>>> "CATCH_AND_RETURN" .
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I agree rename this macro.
>>>>>>>>>>> Do you have any idea? I don't have a good name :-)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Yasumasa
>>>>>>>>>>>
>>>>>>>>>>>> On 2013/09/20 20:10, Staffan Larsen wrote:
>>>>>>>>>>>> Yasuma,
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks for finding and fixing this! I have re-opened the bug.
>>>>>>>>>>>> Your patch looks good to me, but perhaps CATCH_AND_RETURN
>>>>>>>>>>>> should
>>>>>>>>>>>> be renamed CHECK_AND_CLEAR?
>>>>>>>>>>>>
>>>>>>>>>>>> I can sponsor the fix.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> /Staffan
>>>>>>>>>>>>
>>>>>>>>>>>>> On 20 sep 2013, at 12:41, Yasumasa
>>>>>>>>>>>>> Suenaga<yasu at ysfactory.dip.jp> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I encountered this bug:
>>>>>>>>>>>>> JDK-6989981: jstack causes "fatal error: ExceptionMark
>>>>>>>>>>>>> destructor expects no pending exceptions"
>>>>>>>>>>>>>
>>>>>>>>>>>>> I read hs_err and attachListener.cpp, Java heap usage is very
>>>>>>>>>>>>> high and
>>>>>>>>>>>>> it could be OutOfMemoryError in AttachListener::init() .
>>>>>>>>>>>>>
>>>>>>>>>>>>> If JavaCalls::call_special() in AttachListener::init() fail
>>>>>>>>>>>>> which is
>>>>>>>>>>>>> caused by OOME, d'tor of EXCEPTION_MARK (~ExceptionMark) will
>>>>>>>>>>>>> generate
>>>>>>>>>>>>> internal error.
>>>>>>>>>>>>>
>>>>>>>>>>>>> So I make a patch for avoiding crash and attached in this
>>>>>>>>>>>>> email
>>>>>>>>>>>>> (6989981.patch) .
>>>>>>>>>>>>> I'd like to re-open this bug and contribute my patch.
>>>>>>>>>>>>> Could you help me?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --- DETAILS ---
>>>>>>>>>>>>> CHECK macro is used in JavaCalls::call_special() .
>>>>>>>>>>>>>
>>>>>>>>>>>>> ******************
>>>>>>>>>>>>> void AttachListener::init() {
>>>>>>>>>>>>> EXCEPTION_MARK;
>>>>>>>>>>>>>
>>>>>>>>>>>>> :
>>>>>>>>>>>>>
>>>>>>>>>>>>> // Initialize thread_oop to put it into the system threadGroup
>>>>>>>>>>>>> Handle thread_group (THREAD, Universe::system_thread_group());
>>>>>>>>>>>>> JavaValue result(T_VOID);
>>>>>>>>>>>>> JavaCalls::call_special(&result, thread_oop,
>>>>>>>>>>>>> klass,
>>>>>>>>>>>>> vmSymbols::object_initializer_name(),
>>>>>>>>>>>>>
>>>>>>>>>>>>> vmSymbols::threadgroup_string_void_signature(),
>>>>>>>>>>>>> thread_group,
>>>>>>>>>>>>> string,
>>>>>>>>>>>>> CHECK);
>>>>>>>>>>>>>
>>>>>>>>>>>>> KlassHandle group(THREAD,
>>>>>>>>>>>>> SystemDictionary::ThreadGroup_klass());
>>>>>>>>>>>>> JavaCalls::call_special(&result,
>>>>>>>>>>>>> thread_group,
>>>>>>>>>>>>> group,
>>>>>>>>>>>>> vmSymbols::add_method_name(),
>>>>>>>>>>>>> vmSymbols::thread_void_signature(),
>>>>>>>>>>>>> thread_oop, // ARG 1
>>>>>>>>>>>>> CHECK);
>>>>>>>>>>>>> ******************
>>>>>>>>>>>>>
>>>>>>>>>>>>> CHECK macro does not clear pending exception of current
>>>>>>>>>>>>> thread.
>>>>>>>>>>>>> So call_special() fails with runtime exception, d'tor of
>>>>>>>>>>>>> ExceptionMark
>>>>>>>>>>>>> generates fatal error.
>>>>>>>>>>>>>
>>>>>>>>>>>>> ******************
>>>>>>>>>>>>> ExceptionMark::~ExceptionMark() {
>>>>>>>>>>>>> if (_thread->has_pending_exception()) {
>>>>>>>>>>>>> Handle exception(_thread, _thread->pending_exception());
>>>>>>>>>>>>> _thread->clear_pending_exception(); // Needed to avoid
>>>>>>>>>>>>> infinite recursion
>>>>>>>>>>>>> if (is_init_completed()) {
>>>>>>>>>>>>> exception->print();
>>>>>>>>>>>>> fatal("ExceptionMark destructor expects no pending
>>>>>>>>>>>>> exceptions");
>>>>>>>>>>>>> } else {
>>>>>>>>>>>>> vm_exit_during_initialization(exception);
>>>>>>>>>>>>> }
>>>>>>>>>>>>> }
>>>>>>>>>>>>> }
>>>>>>>>>>>>> ******************
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> --- HOW TO REPRODUCE ---
>>>>>>>>>>>>> I also crate testcase of this issue (testcase.tar.gz) . This
>>>>>>>>>>>>> testcase contains
>>>>>>>>>>>>> two modules.
>>>>>>>>>>>>>
>>>>>>>>>>>>> - jvmti: JVMTI agent for this issue. This agent traps
>>>>>>>>>>>>> SIGQUIT and
>>>>>>>>>>>>> calls original (in HotSpot) SIGQUIT handler.
>>>>>>>>>>>>> This signal handler is invoked, MethodEntry event
>>>>>>>>>>>>> callback is
>>>>>>>>>>>>> enabled. MethodEntry generates OutOfMemoryError.
>>>>>>>>>>>>>
>>>>>>>>>>>>> - java : Simple long sleep program.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I can run this testcase in Fedora18 x86_64. See below.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -------
>>>>>>>>>>>>> $ javac java/LongSleep.java
>>>>>>>>>>>>> $ make -C jvmti
>>>>>>>>>>>>> make: Entering directory
>>>>>>>>>>>>> `/data/share/patch/ExceptionMark/testcase/jvmti'
>>>>>>>>>>>>> gcc -I/usr/lib/jvm/java-openjdk/include
>>>>>>>>>>>>> -I/usr/lib/jvm/java-openjdk/include/linux -fPIC -c oome.c
>>>>>>>>>>>>> gcc -shared -o liboome.so oome.o
>>>>>>>>>>>>> make: Leaving directory
>>>>>>>>>>>>> `/data/share/patch/ExceptionMark/testcase/jvmti'
>>>>>>>>>>>>> $ export JAVA_HOME=</path/to/jre>
>>>>>>>>>>>>> $ ./test.sh
>>>>>>>>>>>>> -------
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yasumasa
>>>>>>>>>>>>>
>>>>>>>>>>>>> <6989981.patch><testcase.tar.gz>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Dmitry Samersoff
>>>> Oracle Java development team, Saint Petersburg, Russia
>>>> * I would love to change the world, but they won't give me the
>>>> source code.
>>>
>>
>
More information about the serviceability-dev
mailing list