'Re: RFR(XS): 8158033: notify_tracing() misplaced for intended purpose'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(XS): 8158033: notify_tracing() misplaced for intended purpose
From:       Erik Gahlin <erik.gahlin () oracle ! com>
Date:       2016-05-31 19:47:11
Message-ID: 574DEA3F.9020308 () oracle ! com
[Download RAW message or body]

Looks good!

Not a (R)eviewer

Erik

On 2016-05-27 11:33, Markus Gronlund wrote:
>
> Greetings,
>
> Please review this small fix:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8158033
>
> Webrev: http://cr.openjdk.java.net/~mgronlun/8158033/webrev/ 
> <http://cr.openjdk.java.net/%7Emgronlun/8158033/webrev/>
>
> Description:
>
> The intent when putting in the notify_tracing() hook into debug.cpp 
> (report_java_out_of_memory()) was to intercept a state believed to be 
> a VM termination state, especially when OOME is thrown. Since it is 
> totally valid that Java code catches OOME, and this location actually 
> goes back to Java, this is the wrong location for this hook.
>
> In addition, the hook should not be typed for OOME only, but generic 
> for any exit condition (normal / OOME / crash).
> This should instead have been put into java.cpp (before_exit()) and in 
> VMError.cpp (report_vm_die()).
>
> Thanks
>
> Markus
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=windows-1252"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Looks good! <br>
      <br>
      Not a (R)eviewer<br>
      <br>
      Erik<br>
      <br>
      On 2016-05-27 11:33, Markus Gronlund wrote:<br>
    </div>
    <blockquote cite="mid:39209eae-b854-4de3-b326-147b9d734b43@default"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252">
      <meta name="Generator" content="Microsoft Word 12 (filtered
        medium)">
      <style><!--
/* Font Definitions */
@font-face
	{font-family:"Cambria Math";
	panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
	{font-family:Calibri;
	panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
	{margin:0cm;
	margin-bottom:.0001pt;
	font-size:11.0pt;
	font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
	{mso-style-priority:99;
	color:blue;
	text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
	{mso-style-priority:99;
	color:purple;
	text-decoration:underline;}
span.EmailStyle17
	{mso-style-type:personal-compose;
	font-family:"Calibri","sans-serif";
	color:windowtext;}
.MsoChpDefault
	{mso-style-type:export-only;}
@page WordSection1
	{size:612.0pt 792.0pt;
	margin:70.85pt 70.85pt 70.85pt 70.85pt;}
div.WordSection1
	{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
      <div class="WordSection1">
        <p class="MsoNormal"><span lang="EN-US">Greetings,<o:p></o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US">Please review this small
            fix:<o:p></o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US">Bug: <a
              moz-do-not-send="true"
              href="https://bugs.openjdk.java.net/browse/JDK-8158033"><a \
class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8158033">https://bugs.openjdk.java.net/browse/JDK-8158033</a></a><o:p></o:p></span></p>
  <p class="MsoNormal">Webrev: <a moz-do-not-send="true"
            href="http://cr.openjdk.java.net/%7Emgronlun/8158033/webrev/">http://cr.openjdk.java.net/~mgronlun/8158033/webrev/</a>
  <o:p></o:p></p>
        <p class="MsoNormal"><o:p> </o:p></p>
        <p class="MsoNormal"><span lang="EN-US">Description:<o:p></o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US">The intent when putting
            in the notify_tracing() hook into debug.cpp
            (report_java_out_of_memory()) was to intercept a state
            believed to be a VM termination state, especially when OOME
            is thrown. Since it is totally valid that Java code catches
            OOME, and this location actually goes back to Java, this is
            the wrong location for this hook.<br>
            <br>
            In addition, the hook should not be typed for OOME only, but
            generic for any exit condition (normal / OOME / crash). <br>
            This should instead have been put into java.cpp
            (before_exit()) and in VMError.cpp \
(report_vm_die()).<o:p></o:p></span></p>  <p class="MsoNormal"><span \
                lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US">Thanks<o:p></o:p></span></p>
        <p class="MsoNormal"><span lang="EN-US">Markus<o:p></o:p></span></p>
      </div>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread]
Configure | About | News | Add a list | Sponsored by KoreLogic