On 04/01/2014 02:32 PM, Ehsan Akhgari wrote:
The summary is that I think the mozilla::Atomic API which is modeled after
std::atomic is harmful in that it makes it very non-obvious that you're
dealing with atomic integers. Basically the existing interface makes
mozilla::Atomic look like a normal integer, so that if you see code like:
--mRefCnt;
if (mRefCnt == 0) {
delete this;
}
I think this is a good idea to make the atomic-part of the type appear in
the code. This will at least be a visual hint of threading issue handling.
On the other hand I am against removing these operators, these operators are
convenient as they provide a low cost for instrumenting the current code.
What is possible, is to provide a mozilla::Atomic type with no operator on
it, and provide a lock function:
template <typename T>
mozilla::AtomicHandler<T>
mozilla::lock(mozilla::Atomic<T> &cnt) {
…
}
Such as the AtomicHandler has all the operator to manipulate the Atomic.
Also, we should prevent any other form of construction of the AtomicHandler,
except through the lock function. The above code will then look like this:
--lock(mRefCnt);
if (lock(mRefCnt) == 0) {
delete this;
}
This way, this is more obvious that we might not be doing the right things,
as long as we are careful to refuse AtomicHandler references in reviews.
--
Nicolas B. Pierron
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
