<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 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 href="mailto:jwha@google.com">jwha@google.com</a>> wrote:<br>
><br>
> BUG: <a 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 href="http://cr.openjdk.java.net/~jwha/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 href="mailto:jwha@google.com" target="_blank">jwha@google.com</a></span><br></div><div><br></div></div></div>
</div>