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>
diff -r 9ed97b511b26 src/share/vm/services/attachListener.cpp
--- a/src/share/vm/services/attachListener.cpp Thu Sep 19 11:04:23 2013 -0400
+++ b/src/share/vm/services/attachListener.cpp Sat Sep 21 19:40:17 2013 +0900
@@ -470,7 +470,17 @@
vmSymbols::threadgroup_string_void_signature(),
thread_group,
string,
- CHECK);
+ THREAD);
+
+ if (HAS_PENDING_EXCEPTION) {
+ tty->print_cr("Exception in VM (AttachListener::init) : ");
+ java_lang_Throwable::print(PENDING_EXCEPTION, tty);
+ tty->cr();
+
+ CLEAR_PENDING_EXCEPTION;
+
+ return;
+ }
KlassHandle group(THREAD, SystemDictionary::ThreadGroup_klass());
JavaCalls::call_special(&result,
@@ -479,7 +489,17 @@
vmSymbols::add_method_name(),
vmSymbols::thread_void_signature(),
thread_oop, // ARG 1
- CHECK);
+ THREAD);
+
+ if (HAS_PENDING_EXCEPTION) {
+ tty->print_cr("Exception in VM (AttachListener::init) : ");
+ java_lang_Throwable::print(PENDING_EXCEPTION, tty);
+ tty->cr();
+
+ CLEAR_PENDING_EXCEPTION;
+
+ return;
+ }
{ MutexLocker mu(Threads_lock);
JavaThread* listener_thread = new
JavaThread(&attach_listener_thread_entry);
