RFR(XXS): 8207913: +EnableTracing crashes on jdk8u

Liu, Xin xxinliu at amazon.com
Tue Aug 21 17:40:36 UTC 2018


Hi, David, 

New revision of CR: 
http://cr.openjdk.java.net/~phh/8207913/webrev.02/

It works. Because the main thread only reports Primitive Change Events, which don't use ResourceMark. 
It's not 100% safe because it can't support to dump Klass * and Method * in main thread, or Thread::current() == NULL. 
Does it matter?  In my understanding, main thread sleeps until the java process is about to exit.

Thanks,
--lx

On 8/21/18, 12:38 AM, "David Holmes" <david.holmes at oracle.com> wrote:

    Hi,
    
    On 21/08/2018 5:14 PM, Liu Xin wrote:
    > 
    > hi, Dave,
    > 
    > Thank you reviewing it. Indeed, I intend to submit to jdk8u-dev.
    > 
    > I backtrace the change. here is the original bug.
    > https://bugs.openjdk.java.net/browse/JDK-8032250
    
    Okay. So what you're actually looking for is a backport of this bug 
    fixed in 9:
    
    "8145788: JVM crashes with -XX:+EnableTracing"
    
    https://bugs.openjdk.java.net/browse/JDK-8145788
    
    Can you try the patch in 8u and see if that fixes the issue:
    
    http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/2abdc525b6b4
    
    Thanks,
    David
    
    
    > I found a person mentioned -XX:+EnableTracing was broken for OpenJdk8u 
    > in 2014. He said jdk7 is okay.
    > http://hllvm.group.iteye.com/group/topic/42367
    > 
    > You are right. I found some events do need ResourceMark. It's because 
    > they need to use NEW_RESOURCE_ARRAY in TraceStream. Actually, only 2 
    > cases really need to NEW_RESOURCE_ARRAY, Klass * and Method*.
    > 
    > Is that okay that I use a fixed buffer on stack here?  By using it, I 
    > can safely eliminate the ResourceMark in traceEventClasses.cpp.
    > 
    > diff --git a/src/hotspot/src/share/vm/trace/traceStream.hpp 
    > b/src/hotspot/src/share/vm/trace/traceStream.hpp
    > index 89911ea2..8d4a732f 100644
    > --- a/src/hotspot/src/share/vm/trace/traceStream.hpp
    > +++ b/src/hotspot/src/share/vm/trace/traceStream.hpp
    > @@ -32,6 +32,8 @@
    >   #include "oops/symbol.hpp"
    >   #include "utilities/ostream.hpp"
    > 
    > +const int MAX_LOCAL_BUFFER = 256;
    > +
    >   class TraceStream : public StackObj {
    >    private:
    >     outputStream& _st;
    > @@ -79,29 +81,24 @@ class TraceStream : public StackObj {
    >       _st.print("%s = %f", label, val);
    >     }
    > 
    > -  // Caller is machine generated code located in traceEventClasses.hpp
    > -  // Event<TraceId>::writeEvent() (pseudocode) contains the
    > -  // necessary ResourceMark for the resource allocations below.
    > -  // See traceEventClasses.xsl for details.
    >     void print_val(const char* label, const Klass* const val) {
    > +    char buf[MAX_LOCAL_BUFFER];
    >       const char* description = "NULL";
    > +
    >       if (val != NULL) {
    >         Symbol* name = val->name();
    >         if (name != NULL) {
    > -        description = name->as_C_string();
    > +        description = name->as_C_string(buf, MAX_LOCAL_BUFFER);
    >         }
    >       }
    >       _st.print("%s = %s", label, description);
    >     }
    > 
    > -  // Caller is machine generated code located in traceEventClasses.hpp
    > -  // Event<TraceId>::writeEvent() (pseudocode) contains the
    > -  // necessary ResourceMark for the resource allocations below.
    > -  // See traceEventClasses.xsl for details.
    >     void print_val(const char* label, const Method* const val) {
    > +    char buf[MAX_LOCAL_BUFFER];
    >       const char* description = "NULL";
    >       if (val != NULL) {
    > -      description = val->name_and_sig_as_C_string();
    > +      description = val->name_and_sig_as_C_string(buf, MAX_LOCAL_BUFFER);
    >       }
    >       _st.print("%s = %s", label, description);
    >     }
    > thanks,
    > --lx
    > 
    > 
    > 
    > 
    > On Mon, Aug 20, 2018 at 7:04 PM David Holmes <david.holmes at oracle.com 
    > <mailto:david.holmes at oracle.com>> wrote:
    > 
    >     Hi,
    > 
    >     I dropped jdk8-dev at openjdk.java.net
    >     <mailto:jdk8-dev at openjdk.java.net> and added
    >     jdk8u-dev at openjdk.java.net <mailto:jdk8u-dev at openjdk.java.net> to
    >     the cc line.
    > 
    >     On 21/08/2018 8:38 AM, Liu, Xin wrote:
    >      > Hi, Runtime develop,
    >      >
    >      > Could you review this change basing on jdk8u?
    >      > Problem: https://bugs.openjdk.java.net/browse/JDK-8207913
    >      > Webrev: http://cr.openjdk.java.net/~phh/8207913/webrev.01/
    >     <http://cr.openjdk.java.net/%7Ephh/8207913/webrev.01/>
    >      >
    >      > JDK8u has this problem because Thread::current() is NULL when it
    >     handles global arguments at startup. OracleJDK8 doesn’t suffer from
    >     this issue.
    >      > I found jdk12’s counterpart jfrEventClasses.hpp doesn’t have
    >     ResourceMark.  Removing it works for both with/without ttyLocker.
    > 
    >     The ResourceMark has been there since the file was introduced in JDK 8.
    >     What has changed to cause this problem and when? If we remove the RM
    >     then we have to be sure no events that get traced will in fact rely on
    >     there being a RM present.
    > 
    >      > We will check in jdk8u/hotspot if reviewers approve it.
    > 
    >     There is a specific approval process that has to be followed after the
    >     change has been Reviewed.
    > 
    >     http://openjdk.java.net/projects/jdk8u/approval-template.html
    > 
    >     Thanks,
    >     David
    > 
    >      > Thanks,
    >      > --lx
    >      >
    > 
    



More information about the hotspot-runtime-dev mailing list