The subject of this post is intentionally chosen to make you want to read
this. :-)
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;
}
You won't immediately think about checking the type of mRefCnt (the
refcount case being just an example here of course), which makes it hard to
spot that there is a thread safety bug in this code. Given that I and
three experienced Gecko hackers fell prey to this issue (bug 985878 and bug
987667), I thought about the underlying problem a bit, and managed to
convince myself that it's a bad idea for us to pretend that atomic integers
can be treated the same way as non-atomic integers.
So, over in bug 987887 I'm proposing to remove all of the methods on
mozilla::Atomic except for copy construction and assignment and replace
them with global functions that can operate on the atomic type. <atomic>
has a number of global functions that can operate on std::atomic types <
http://en.cppreference.com/w/cpp/atomic> and we can look into implementing
similar ones (for example, mozilla::AtomicFetchAdd,
mozilla::AtomicExchange, etc. -- names to be bikeshedded outside of this
thread!)
There is an extensive discussion on bug 987887 about this. The proponents
of this change have argued that it makes it easier to spot that a given
type is an atomic one because the type is explicitly treated differently
than normal built-in types, even at the lack of a bit of convenience that
the member operators provide. The opponents of this change have argued
that the alternative is just as error prone as what we have today, and that
it diverges us from being compatible with the std::atomic type (in answer
to which I have said that is a good thing, since std::atomic is affected by
the same problem and I think is a bad idea for us to copy it.)
I am of course biased towards my side of the argument, so please read the
bug to get a better understanding of people's positions.
What do people feel about my proposal? Do you think it improves writing
and reviewing thread safe code to be less error prone?
Cheers,
--
Ehsan
<http://ehsanakhgari.org/>
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
