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