On Wednesday 2014-04-02 12:50 +0900, Mike Hommey wrote:
> On Tue, Apr 01, 2014 at 08:34:01PM -0400, Ehsan Akhgari wrote:
> > That is the code after I changed it.  :-)  Here is the original patch which
> > introduced this bug:
> > <https://bug935778.bugzilla.mozilla.org/attachment.cgi?id=8385072>  The
> > thread safety issue was _not_ there before that patch.
> 
> I'm not saying there was a thread safety issue before. I'm saying that in
> your example, there is largely the same thread safety issue whether the
> code uses a plain int or an atomic one. The use of atomics is _not_ hiding
> this bug.

That's not the issue.  The issue is that the syntax is hiding the
use of atomics.

And whether the code would have had a thread-safety bug if it hadn't
been using atomics is also not relevant; lots of single-threaded
code has "thread-safety bugs" if you magically decide to violate its
threading invariants and use the same object on multiple threads
when it wasn't designed for that.

The issue here is whether this particular way of writing threadsafe
code leads people modifying that code to make mistakes because they
don't even notice that it's threadsafe code.

> > >As I said in the bug, all this is saying is that thread safety is hard,
> > >and atomics are merely one of the tools to achieve thread safety. They
> > >are not a magic wand that fixes thread safety.
> > 
> > Did you read <https://bugzilla.mozilla.org/show_bug.cgi?id=987887#c20>?
> 
> Did you read my answer to that comment?

I think you're continuing to ignore the fact that using operator
overloading obscures what those operators are doing underneath, and
when that difference is important to the reader of the code,
overloading might not be a good idea.

> > Continuing to reduce my argument here to "atomics should be a magic wand" is
> > really depressing.
> > 
> > >I also think that making their API not have operators like std::atomic
> > >has would bring a false sense of security to people writing code using
> > >them, because it would supposedly be less confusing when it really
> > >wouldn't.
> > 
> > Can you please give an example of that concern?  I'm not sure if I follow.
> 
> Just take the original patch which introduced the bug. I seriously don't
> believe if the patch had been using FetchAndAdd instead of ++/-- the issue
> would have had more chance of being caught at review. And I don't
> believe it would make people more likely to make atomic-using code
> thread safe.

You might claim that the increased chance of being caught is small,
but claiming it's zero is laughable.

-David

-- 
𝄞   L. David Baron                         http://dbaron.org/   𝄂
𝄢   Mozilla                          https://www.mozilla.org/   𝄂
             Before I built a wall I'd ask to know
             What I was walling in or walling out,
             And to whom I was like to give offense.
               - Robert Frost, Mending Wall (1914)

Attachment: signature.asc
Description: Digital signature

_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to