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.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.
