RFR(XXS): 8207913: +EnableTracing crashes on jdk8u
David Holmes
david.holmes at oracle.com
Wed Aug 22 04:04:27 UTC 2018
Hi,
On 22/08/2018 3:40 AM, Liu, Xin wrote:
> Hi, David,
>
> New revision of CR:
> http://cr.openjdk.java.net/~phh/8207913/webrev.02/
If this is a clean direct backport of the change in 9 then this should
be treated as a backport approval request. Close 8207913 as a duplicate
and just import the changeset for 8145788 and request approval for that
backport.
You could use this bug to add the test but that would be a separate
enhancement request (plus code review plus approval request). If you did
add the test it would be preferable to do it all in Java using the test
library and ProcessTools (I think we have that in 8u!) rather than a
shell script.
> 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.
There are lots of things that can fail if they are used too early in the
VM initialization process - this is no different. Obviously to use a
ResourceMark you must have an attached thread, and there is a period of
time during VM initialization where the main thread is not yet attached
and so Thread::current() will be NULL.
FYI I'm on vacation after today so I won't be able to continue this
discussion.
Cheers,
David
> 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