RFR(s): 8037112: gc/g1/TestHumongousAllocInitialMark.java caused SIGSEGV

[Per Liden]

Bengt reviewed this off-line and had one comment, which was that we 
should also stop G1's string dedup thread if it's running. The dedup 
thread is not affected by the original problem so stopping it is 
strictly not needed, but for completness we still want to do it.

Updated webrev:
http://cr.openjdk.java.net/~pliden/8037112/webrev.3/

Diff against previous webrev:
http://cr.openjdk.java.net/~pliden/8037112/webrev.3-diff_2vs3/

/Per

On 04/08/2014 11:27 AM, Per Liden wrote:
> Hi,
>
> Here's an updated webrev which fixes the potential dead lock issue I
> found. I've taken a whole new approach to this problem. Instead of
> joining/leaving the STS as protection from shutdown I've added a
> CollectedHeap::stop() which is called in the shutdown paths (in
> before_exit()) to stop and take down the concurrent threads in a
> controlled manner. This feel like a much cleaner approach and less error
> prone as it doesn't require any future log printouts to be guarded by
> join/leave.
>
> Webrev: http://cr.openjdk.java.net/~pliden/8037112/webrev.2/
>
> cheers,
> /Per
>
> On 04/04/2014 06:14 PM, Per Liden wrote:
>> Hmm, I discovered a potential dead lock with this approach, so please
>> hold reviews on this for now.
>>
>> Sorry!
>> /Per
>>
>> On 04 Apr 2014, at 14:28, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>>
>>> Per,
>>>
>>> On Friday 04 April 2014 14.13.56 Per Liden wrote:
>>>> Thanks Mikael!
>>>>
>>>> Updated webrev: http://cr.openjdk.java.net/~pliden/8037112/webrev.1/
>>>
>>> Looks good.
>>> /Mikael
>>>
>>>>
>>>> /Per
>>>>
>>>> On 04/03/2014 03:59 PM, Mikael Gerdin wrote:
>>>>> Hi Per,
>>>>>
>>>>> On Thursday 03 April 2014 13.15.12 Per Liden wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Could I please have this bugfix reviewed.
>>>>>>
>>>>>> Summary: G1's ConcurrentMarkThread tries to use the gclog_or_tty when
>>>>>> the JVM is about to exit and the gclog_or_tty instance has been
>>>>>> destroyed. To prevent this, the ConcurrentMarkThread needs to join
>>>>>> the
>>>>>> SuspendibleThreadSet before accessing gclog_or_tty.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8037112
>>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8037112/webrev.0/
>>>>>
>>>>> While I realize that this code is still semantically correct I
>>>>> think it
>>>>> reads a bit strange. In my mind if we have a
>>>>> if (...) {} else if (...) {} else {} block the checks in the "if" and
>>>>> "else
>>>>> if" should usually somehow related checks.
>>>>>
>>>>>   282       if (!cm()->has_aborted()) {
>>>>>   283         g1_policy->record_concurrent_mark_cleanup_completed();
>>>>>   284       } else if (G1Log::fine()) {
>>>>>   285         gclog_or_tty->date_stamp(PrintGCDateStamps);
>>>>>   286         gclog_or_tty->stamp(PrintGCTimeStamps);
>>>>>   287         gclog_or_tty->print_cr("[GC concurrent-mark-abort]");
>>>>>   288       }
>>>>>
>>>>> I would prefer if it was structured the other way around:
>>>>> if (cm()->has_aborted()) {
>>>>>
>>>>>     if (G1Log::fine()) { ... }
>>>>>
>>>>> else {
>>>>>
>>>>>     g1_policy->record_concurrent_mark_cleanup_completed();
>>>>>
>>>>> }
>>>>>
>>>>> The rest of the change looks good.
>>>>>
>>>>> /Mikael
>>>>>
>>>>>> Thanks!
>>>>>> /Per
>>>>>>
>>>>>> Btw, I created a separate RFE, JDK-8039147, to do a little bit of
>>>>>> cleanup of the SuspendibleThreadSet and how it's used. Will send that
>>>>>> out as separate review request shortly.
>>>
>>
>