<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Jungwoo,<br>
<br>
I reworked the patch just a bit. <br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~jmasa/8141356/webrev.00/">http://cr.openjdk.java.net/~jmasa/8141356/webrev.00/</a><br>
<br>
I moved the definition of stop() to the .cpp file. The .cpp<br>
file already had an include of concurrentMarkSweepThread.hpp<br>
and I think we're trying to minimize the include of .hpp files<br>
into other .hpp files.<br>
<br>
I also added the guard<br>
<pre><span class="new">#if INCLUDE_ALL_GCS
There is a build that does not include all the GC
and the guard is needed in that build.
Is this version all right with you?
Jon
</span></pre>
<br>
<div class="moz-cite-prefix">On 11/8/2015 9:55 AM, Jungwoo Ha wrote:<br>
</div>
<blockquote
cite="mid:CA+n_jhjC-QU189ZVNanwDVK6LVTV0eerG4gu2f-CbH-9x_Yiww@mail.gmail.com"
type="cite">
<div dir="ltr">Thanks for the review, Kim. --Jungwoo</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Nov 5, 2015 at 10:40 PM, Kim
Barrett <span dir="ltr"><<a moz-do-not-send="true"
href="mailto:kim.barrett@oracle.com" target="_blank">kim.barrett@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"><span
class="">On Nov 3, 2015, at 4:14 PM, Jungwoo Ha <<a
moz-do-not-send="true" href="mailto:jwha@google.com"><a class="moz-txt-link-abbreviated" href="mailto:jwha@google.com">jwha@google.com</a></a>>
wrote:<br>
><br>
> BUG: <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8141356"
rel="noreferrer" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8141356</a><br>
> Webrev: <a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ejwha/8141356/webrev.00/"
rel="noreferrer" target="_blank">http://cr.openjdk.java.net/~jwha/8141356/webrev.00/</a><br>
><br>
> Inside before_exit(), it says<br>
><br>
> // Stop concurrent GC threads<br>
> Universe::heap()->stop();<br>
><br>
> but GenCollectedHeap never implemented stop() method,
and thus calling empty SharedHeap::stop() method.<br>
> This causes CMS threads to run during the VM
termination and ends up crashing.<br>
> ConcurrentMarkSweepThread::stop() is currently a dead
code and never gets called.<br>
> The patch just implemented GenCollectedHeap::stop()
to call ConcurrentMarkSweepThread::stop().<br>
<br>
</span>I was going to ask why CMS doesn’t have its own heap
class where the implementation of stop should be placed.<br>
But I see that CMS is already significantly entangled with
GenCollectedHeap. Given that, change looks good.<br>
<br>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
<div class="gmail_signature">
<div dir="ltr">
<div><span style="font-size:12.8000001907349px">Jungwoo Ha |
Java Platform Team | <a moz-do-not-send="true"
href="mailto:jwha@google.com" target="_blank">jwha@google.com</a></span><br>
</div>
<div><br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>