JDK-6989981: jstack causes "fatal error: ExceptionMark destructor expects no pending exceptions"
Yasumasa Suenaga
yasu at ysfactory.dip.jp
Fri Oct 4 17:32:05 PDT 2013
Hi David,
> Will the tool fail differently now compared to the VM aborting?
With or without a patch, we will get same result from serviceability tools.
In tools side, they cannot connect to UNIX domain socket.
So we wiil get messages in TTY of tools as following:
-----------
1418: Unable to open socket file: target process not responding or HotSpot VM not loaded
The -F option can be used when the target process is not responding
-----------
Thanks,
Yasumasa
On 2013/10/04 9:14, David Holmes wrote:
> 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