RFR(S): 8141356: Explicitly stop CMS threads during VM termination.

[Jungwoo Ha]

Thanks for the fix. Looks good to me.
On Nov 11, 2015 11:56 PM, "Jon Masamitsu" <jon.masamitsu at oracle.com> wrote:

> Jungwoo,
>
> I reworked the patch just a bit.
>
> http://cr.openjdk.java.net/~jmasa/8141356/webrev.00/
>
> I moved the definition of stop() to the .cpp file.  The .cpp
> file already had an include of concurrentMarkSweepThread.hpp
> and I think we're trying to minimize the include of .hpp files
> into other .hpp files.
>
> I also added the guard
>
> #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
>
>
> On 11/8/2015 9:55 AM, Jungwoo Ha wrote:
>
> Thanks for the review, Kim. --Jungwoo
>
> On Thu, Nov 5, 2015 at 10:40 PM, Kim Barrett <kim.barrett at oracle.com>
> wrote:
>
>> On Nov 3, 2015, at 4:14 PM, Jungwoo Ha <jwha at google.com> wrote:
>> >
>> > BUG: https://bugs.openjdk.java.net/browse/JDK-8141356
>> > Webrev: http://cr.openjdk.java.net/~jwha/8141356/webrev.00/
>> >
>> > Inside before_exit(), it says
>> >
>> >   // Stop concurrent GC threads
>> >   Universe::heap()->stop();
>> >
>> > but GenCollectedHeap never implemented stop() method, and thus calling
>> empty SharedHeap::stop() method.
>> > This causes CMS threads to run during the VM termination and ends up
>> crashing.
>> > ConcurrentMarkSweepThread::stop() is currently a dead code and never
>> gets called.
>> > The patch just implemented GenCollectedHeap::stop() to call
>> ConcurrentMarkSweepThread::stop().
>>
>> I was going to ask why CMS doesn’t have its own heap class where the
>> implementation of stop should be placed.
>> But I see that CMS is already significantly entangled with
>> GenCollectedHeap.  Given that, change looks good.
>>
>>
>
>
> --
> Jungwoo Ha | Java Platform Team | jwha at google.com
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20151112/35e7aaac/attachment.htm>