RFR: 8010151 nsk/regression/b6653214 fails "assert(snapshot != NULL) failed: Worker should not be started"

[David Holmes]

Looks good to me.

David

On 10/04/2013 3:14 AM, Zhengyu Gu wrote:
> Updated webrev based on David's comment:
>
> http://cr.openjdk.java.net/~zgu/8010151/webrev.02/
>
> Thanks,
>
> -Zhengyu
>
> On Apr 9, 2013, at 9:45 AM, Zhengyu Gu wrote:
>
>> Hi David,
>>
>> On Apr 8, 2013, at 8:44 PM, David Holmes wrote:
>>
>>> Hi Zhengyu,
>>>
>>> On 9/04/2013 6:44 AM, Zhengyu Gu wrote:
>>>> Second round.
>>>>
>>>> It fixed a racing condition when NMT is being shutdown while worker thread just started. The race causes get_snapshot() to return NULL, that triggers the assertion.
>>>
>>> I can't see where get_snapshot() is now called such that we know we have a snapshot when calling start_worker() ??
>>>
>> Now, snapshot is passed to MemTrackWorker from MemTracker::start(), the method ensures that snapshot is properly created and initialized before calling start_worker().
>>
>>>> It also fixed a few volatile pointers that were mistakenly declared as volatile objects.
>>>
>>> There is some further cleanup here:
>>>
>>> 209 // delete all pooled recorders
>>> 210 void MemTracker::delete_all_pooled_recorders() {
>>> 211   // free all pooled recorders
>>> 212   volatile MemRecorder* cur_head = _pooled_recorders;
>>>
>>> cur_head declaration is no longer correct.
>>>
>>
>> Thanks for finding this, I will fix it.
>>
>> Thanks,
>>
>> -Zhengyu
>>
>>> David
>>> -----
>>>
>>>>
>>>> Bug:       http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8010151
>>>> Webrev: http://cr.openjdk.java.net/~zgu/8010151/webrev.01/
>>>>
>>>> Tests:
>>>>   - JPRT
>>>>   - vm.quick.testlist on Linux x86 and Windows x64
>>>>   - The approach described by Christian in bug report. I were able to reproduce on prior the fix, and it is no longer reproducible after.
>>>>
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>
>>
>