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

[David Holmes]

On 9/04/2013 11:45 PM, 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().

Sorry I misunderstood the original problem. Originally you had:

     _snapshot = new (std::nothrow)MemSnapshot();
     if (_snapshot != NULL) {
       if (!_snapshot->out_of_memory() && start_worker()) {

and then later the worker would call get_snapshot(). In the above the 
_snapshot can not be NULL, but the problem was that it was NULL by the 
time get_snapshot() was invoked. I presume this is because the shutdown 
process set it to NULL. So now you avoid this by passing it directly to 
the worker - so get_snapshot doesn't enter into it at all.

David

>>> 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
>>>
>