> On 20 sep 2013, at 16:49, Yasumasa Suenaga <y...@ysfactory.dip.jp> wrote:
>
>> On 2013/09/20 23:34, Staffan Larsen wrote:
>>
>>> On 20 sep 2013, at 16:24, Yasumasa Suenaga<y...@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<y...@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<y...@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>
>
