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);

Reply via email to