Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation for Acquire/Release handshake when dropping Arcs is misleading #62230

Closed
mzabaluev opened this issue Jun 29, 2019 · 1 comment
Closed
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mzabaluev
Copy link
Contributor

mzabaluev commented Jun 29, 2019

#41730 and previous changes added comments describing the reasoning for the Acquire fence synchronizing with the Release counter decrement before the content of Arc is deallocated. It refers to Boost documentation as an authoritative source. Unfortunately, even that gets its wrong: there is nothing in the C11/C++ standards (which LLVM adheres to) that guarantees ordering of non-atomic operations in presence of atomic fences. The fences in the current implementation may still be needed to e.g. order operations between strong and weak counts. However, the burden of ensuring that a newly allocated data block cannot be accessed by other threads which use past allocations at the same location lies entirely on the allocator.

The documentation of Alloc and GlobalAlloc may also need to be updated with the obligation to provide this guarantee for the implementations of thread-safe allocators.

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 29, 2019
@mzabaluev
Copy link
Contributor Author

mzabaluev commented Jun 30, 2019

Hmm, I think I misread the rules about intra-thread sequence points in the standard. Withdrawing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants