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

[Liu Xin]

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

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>
wrote:

> Hi,
>
> I dropped jdk8-dev at openjdk.java.net and added
> 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/
> >
> > 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
> >
>