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

[Dean Long]

If the analysis by Christian Törnqvist is correct then you should be 
able to reproduce the problem by adding sleeps
where he suggested.  You will then probably discover that "volatile" 
does not solve the problem.  I think there are
still races and the code is not MT-safe.

This may not solve all the races, but you could try something like:

bool MemTracker::start_worker(MemSnapshot *snapshot) {
   assert(_worker_thread == NULL, "Just Check");
   _worker_thread = new (std::nothrow) MemTrackWorker(snapshot);

so that the worker thread doesn't need to call get_snapshot().

dl

On 4/3/2013 3:21 PM, Zhengyu Gu wrote:
> Thanks for pointing it out. Yes, it should be a volatile pointer.
>
> -Zhengyu
>
> On 4/3/2013 2:40 PM, Coleen Phillimore wrote:
>>
>> Is the _snapshot volatile or what it points to is volatile? The code 
>> you have is what it points to.
>>
>> I think you want:
>>
>> *!    static_   _MemSnapshot*   volatile  _snapshot;*
>>
>> Coleen
>>
>> On 04/03/2013 01:59 PM, Zhengyu Gu wrote:
>>> The assertion appears due to snapshot variable still is not visible 
>>> to worker thread, so make snapshot volatile.
>>>
>>> NMT creates snapshot before starting NMT worker thread and the 
>>> snapshot is released by worker thread during shutdown, so the 
>>> assertion should not fail. I can not reproduce the problem 
>>> with/without volatile modifier.
>>>
>>>
>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8010151
>>> Webrev: http://cr.openjdk.java.net/~zgu/8010151/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Ezgu/8010151/webrev.00/>
>>>
>>> Test:
>>>   nsk/regression/b6653214  on Windows x64
>>>
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130403/6acb25bb/attachment-0001.html