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.
/Staffan
On 22 sep 2013, at 01:51, Dmitry Samersoff <dmitry.samers...@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<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>
>>>>
>>>
>>
>
>
> --
> 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.
