Re: [Boost-users] [atomic] ThreadSanitizer reports a data race when using the code from "reference counting" example

Subject: Re: [Boost-users] [atomic] ThreadSanitizer reports a data race when using the code from "reference counting" example
From: Adam Romanek (romanek.adam_at_[hidden])
Date: 2014-07-01 07:34:16

On 01.07.2014 10:49, Andrey Semashev wrote:
> On Tue, Jul 1, 2014 at 12:41 PM, Adam Romanek wrote:
>> On 01.07.2014 10:36, Andrey Semashev wrote:
>>>
>>> On Tue, Jul 1, 2014 at 12:20 PM, Andrey Semashev
>>> wrote:
>>>>
>>>> On Tue, Jul 1, 2014 at 11:20 AM, Adam Romanek
>>>> wrote:
>>>>>
>>>>> Andrey,
>>>>>
>>>>> If you don't mind, please have a look at the conversation that I started
>>>>> on
>>>>> boost-users list [1] a few days back. Since you're the maintainer of
>>>>> Boost.Atomic I'd like to know your opinion on this issue. Or maybe I
>>>>> should
>>>>> repost this on boost-dev list?
>>>>
>>>>
>>>> I don't follow users list, mostly because I can't even keep up with
>>>> the developers list. So it's better to reach me on the dev list.
>>>>
>>>> The code in the StackOverflow topic indeed contains a potential
>>>> problem. The fetch_sub(release) operation prevents previous operations
>>>> from sinking past it and atomic_thread_fence(acquire) prevents the
>>>> following operations from floating above it. However, these two
>>>> barriers themselves can be reordered creating a window where
>>>> operations before fetch_sub(release) and after
>>>> atomic_thread_fence(acquire) may overlap. I believe, this is permitted
>>>> on both the compiler and hardware levels. This doesn't mean that's
>>>> what is actually happening and being flagged by TSan; it is still
>>>> possible that some TSan bug is exposed. The fact that replacing
>>>> atomic_thread_fence(acquire) with fetch_add(acquire) speaks in favor
>>>> of the bug suspicion.
>>>
>>>
>>> Actually, no, fetch_add(acquire) is significantly different from the
>>> fence. It operates on the same object as the previous
>>> fetch_sub(release), so it must observe its effects and cannot be
>>> reordered.
>>>
>>
>> After reading your first response I thought you're suggesting that the
>> "reference counting" example in Boost.Atomic docs is broken. But hopefully
>> we agree that it's fine, right? :)
>
> The example in the docs is indeed not correct. I believe, it should be
> corrected to use acq_rel memory order in intrusive_ptr_release. The
> fence can be removed then. I'll fix it as soon as I have some time.
>
>> So I guess this is a false positive from TSan, as suggested by others on SO.
>> I'll ask this on TSan's mailing list, lets see what they say.
>
> Ok, let me know if there are other considerations from TSan devs.
>

See above for my private conversation with Andrey Semashev on this topic 
(he does not follow the boost-users mailing list).

Andrey says that the "reference counting example" from Boost.Atomic docs 
is broken. What do you think about this?

WBR,
Adam Romanek