Thanks Marcus for taking a look. /Markus
-----Original Message----- From: Marcus Larsson Sent: den 12 januari 2016 15:29 To: Markus Gronlund; Yasumasa Suenaga; David Holmes Cc: serviceability-dev@openjdk.java.net Subject: Re: PING: RFR: JDK-8145788: JVM crashes with -XX:+EnableTracing Hi Markus, Looks good! Thanks, Marcus On 01/12/2016 10:34 AM, Markus Gronlund wrote: > Thanks David and Yasumasa, > > I will then proceed with checking in my suggestion. > > Thanks again > Markus > > -----Original Message----- > From: Yasumasa Suenaga [mailto:yasue...@gmail.com] > Sent: den 11 januari 2016 11:09 > To: Markus Gronlund; David Holmes > Cc: serviceability-dev@openjdk.java.net > Subject: Re: PING: RFR: JDK-8145788: JVM crashes with > -XX:+EnableTracing > > Hi Markus, David, > > I agree the solution of Markus too! > > > Thanks, > > Yasumasa > > > On 2016/01/11 18:48, Markus Gronlund wrote: >> Hi David, >> >> Thanks for pointing to the locked branch as well. >> >> Thanks to the bootstrap aware logic in defaultStream::hold(intx writer_id) >> this should be ok as is: >> >> ttyLocker will call the hold() member function which defaults to single >> threaded behavior during bootstrap (DefaultStream::NOWRITER). This is >> accomplished by checking the availability of both ttyLock as well as >> Thread::current_or_null(). >> >> Cheers >> Markus >> >> -----Original Message----- >> From: David Holmes >> Sent: den 11 januari 2016 02:39 >> To: Markus Gronlund; Yasumasa Suenaga >> Cc: serviceability-dev@openjdk.java.net >> Subject: Re: PING: RFR: JDK-8145788: JVM crashes with >> -XX:+EnableTracing >> >> Hi Markus, >> >> Thanks very much for stepping in on this one. I like your solution to this >> particular problem. >> >> Aside: I noticed in writeEvent there is also UseLockedTracing and I'm >> wondering if that will also lead to an assertion failure due to no current >> thread? >> >> Thanks, >> David >> >> On 11/01/2016 9:25 AM, Markus Gronlund wrote: >>> Hi Yasumasa, >>> >>> Apologies for the delay in getting a response to you. >>> >>> Thanks for reporting and attempting to fix this issue. >>> >>> I have investigated this a bit as well as trying out your suggested patch. >>> >>> I must admit it is hard to get this right at this early stage of the VM, >>> and though I appreciated your effort for resolution, the patch suggestion >>> will not work out of the box unfortunately. >>> >>> I have summarized some of my findings and reasoning here: >>> >>> http://cr.openjdk.java.net/~mgronlun/8145788/notes.txt >>> >>> As you can see, Thread::current() cannot be called at this stage. If we >>> update to instead call Thread::current_or_null(), you will now run into >>> problems in the interplay between an explicit delete of a Cheap allocated >>> ResourceArea and the subsequent ResourceMark destructor. Here, the rm >>> destructor asserts since it expects _nesting_level > 0, but that data has >>> already been invalidated. >>> >>> You could accomplish all of this by doing: >>> >>> + bool thread_has_resource = Thread::current_or_null() != NULL; >>> + ResourceArea *area = thread_has_resource >>> + ? Thread::current()->resource_area() >>> + : new >>> (mtTracing)ResourceArea(Chunk::non_pool_size); >>> + { >>> + ResourceMark rm(area); >>> + if (UseLockedTracing) { >>> + ttyLocker lock; >>> + writeEventContent(); >>> + } else { >>> + writeEventContent(); >>> + } >>> + } >>> + >>> + if (!thread_has_resource) { >>> + delete area; >>> } >>> >>> >>> However, it is getting a bit complex. In addition, now every trace >>> statement needs to check Thread::current_or_null().. >>> >>> If you look at my reasoning in the second part, I think we can fix this in >>> an easier way. >>> >>> You can find my suggestion as a webrev here: >>> >>> http://cr.openjdk.java.net/~mgronlun/8145788/ >>> >>> Please try the patch out to see if you are ok with it. >>> >>> Thanks for your patience >>> >>> Best regards >>> Markus >>> >>> >>> >>> -----Original Message----- >>> From: Yasumasa Suenaga [mailto:yasue...@gmail.com] >>> Sent: den 10 januari 2016 14:03 >>> To: serviceability-dev@openjdk.java.net >>> Subject: PING: RFR: JDK-8145788: JVM crashes with -XX:+EnableTracing >>> >>> Ping: What do you think about this issue? >>> >>> On 2016/01/06 15:36, David Holmes wrote: >>>> Pinging the serviceability tracing experts please! >>>> >>>> David >>>> >>>> On 24/12/2015 12:25 AM, Yasumasa Suenaga wrote: >>>>> Hi David, >>>>> >>>>>>> 1. Initialize JavaThread before calling apply_ergo() in >>>>>>> create_vm(). >>>>>> That is not likely to be an option - it would likely be far too >>>>>> disruptive to the initialization sequence. >>>>> Agree. Thus I choose 2. >>>>> >>>>>> We will have to wait for the tracing experts to have a good look >>>>>> at this. >>>>> I'm waiting that the tracing experts join this discussion. >>>>> >>>>> >>>>> Thanks, >>>>> >>>>> Yasumasa >>>>> >>>>> >>>>> On 2015/12/23 13:20, David Holmes wrote: >>>>>> Hi Yasumasa, >>>>>> >>>>>> On 23/12/2015 11:49 AM, Yasumasa Suenaga wrote: >>>>>>> Hi David, >>>>>>> >>>>>>> I've added callstack when JVM crashed: >>>>>>> >>>>>>> https://bugs.openjdk.java.net/browse/JDK-8145788?focusedCommentI >>>>>>> d >>>>>>> = >>>>>>> 1 >>>>>>> 3880225&page=com.atlassian.jira.plugin.system.issuetabpanels:com >>>>>>> m >>>>>>> e >>>>>>> n >>>>>>> t-tabpanel#comment-13880225 >>>>>> Thanks for that. >>>>>> >>>>>>> This crash occurrs in Arguments::apply_ergo() in Threads::create_vm(). >>>>>>> apply_ergo() calls before JavaThread initialization. >>>>>>> >>>>>>> I think that TraceEvent can avoid crash with two approach: >>>>>>> >>>>>>> 1. Initialize JavaThread before calling apply_ergo() in >>>>>>> create_vm(). >>>>>> That is not likely to be an option - it would likely be far too >>>>>> disruptive to the initialization sequence. >>>>>> >>>>>>> 2. Avoid crash at TraceEvent when it is called before >>>>>>> JavaThread initialization. >>>>>> Or don't call it at all. >>>>>> >>>>>> We will have to wait for the tracing experts to have a good look >>>>>> at this. We end up in code that is not expecting to be run before >>>>>> more of the VM is initialized, so we have to look at what else >>>>>> may be being assumed and then decide whether to handle the situation or >>>>>> avoid it. >>>>>> >>>>>> Thanks, >>>>>> David >>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> Yasumasa >>>>>>> >>>>>>> >>>>>>> On 2015/12/22 21:19, David Holmes wrote: >>>>>>>> On 19/12/2015 1:50 AM, Yasumasa Suenaga wrote: >>>>>>>>> Hi all, >>>>>>>>> >>>>>>>>> I encountered JVM crash when I passed -XX:+EnableTracing. >>>>>>>>> >>>>>>>>> I checked core image, it crashed in >>>>>>>>> EventBooleanFlagChanged::writeEvent() >>>>>>>>> which is called by Arguments::apply_ergo() because thread >>>>>>>>> had not been >>>>>>>>> initialized. (JVM seems to log changing GC algorithm >>>>>>>>> to >>>>>>>>> G1.) >>>>>>>> This seems like a logic error to me - something is trying to >>>>>>>> happen too early during VM initialization. We need to look at >>>>>>>> exactly what we are trying to do here, not just work around the crash. >>>>>>>> >>>>>>>> David >>>>>>>> ----- >>>>>>>> >>>>>>>>> writeEvent() uses ResourceMark. Default constructor of >>>>>>>>> ResourceMark uses >>>>>>>>> ResourceArea in current thread. So ResourceMark in >>>>>>>>> writeEvent() should >>>>>>>>> pass valid ResourceArea. >>>>>>>>> >>>>>>>>> I think this issue is in traceEventClasses.xsl . >>>>>>>>> However, my environment (GCC 5.3.1 on Fedora23) cannot build >>>>>>>>> it because -Werror=maybe-uninitialized was occurred. >>>>>>>>> So I also fixed them together. >>>>>>>>> >>>>>>>>> I've uploaded webrev. Could you review it? >>>>>>>>> >>>>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8145788/webrev.00/ >>>>>>>>> >>>>>>>>> I'm jdk9 committer, however I cannot access JPRT. >>>>>>>>> So I need a sponsor. >>>>>>>>> >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> >>>>>>>>> Yasumasa >>>>>>>>>
