4 ms·

Having reviewed even just a few diffs with Synchronized since joining Facebook, it absolutely should not be part of the standard library (nor, I would argue, pa

Having reviewed even just a few diffs with Synchronized since joining Facebook, it absolutely should not be part of the standard library (nor, I would argue, part of folly). I have found far too many cases where engineers have threading issues in their code because they're saving references to objects after .lock() has returned. Writing thread-safe code isn't easy, and it isn't made easier by libraries that offer footguns like Synchronized. It's easier to get it right with compiler thread annotations than with Synchronized.

ot 5y ago
How do compiler thread annotations solve the problem of persisting references to the object? Is static analysis able to detect that? Seems a hard problem (basically escape analysis). Synchronized cannot guarantee thread-safety, but in my experience wrappers that do not allow unsynchronized access to the object are better than the alternative. Annotations are just another way to guard access.
- jemfinch 5y ago
  They don't, but C++ programmers especially are familiar with not letting inappropriate things escape scopes; they're much less familiar with not letting inappropriate things escape _expressions_. I've reviewed over a half million lines of code from engineers of every level of experience, and I've seen far more bugs from the latter than the former. For instance, a former employer had a function similar to folly::get_default, except it returned the default by reference, and engineers were always running afoul of lifetimes using that function. (It was especially egregious with std::string temporaries returned from string literal defaults.) Thread annotations also effectively prevent unsynchronized access to the object, but they do it in a way that programmers make fewer mistakes with, and they do it with a less error prone mental model: mutexes are seen as guards of _class invariants_, not merely as guards of _data members_. Apart from encouraging a superior mental model, thread annotations also encourage a one-mutex-per-class model which I've found far more successful and error prone (both in code I've reviewed and in code I've written) over a one-mutex-per-member model which inevitably (in my experience) results in fewer bugs.

ot 5y ago

How do compiler thread annotations solve the problem of persisting references to the object? Is static analysis able to detect that? Seems a hard problem (basically escape analysis). Synchronized cannot guarantee thread-safety, but in my experience wrappers that do not allow unsynchronized access to the object are better than the alternative. Annotations are just another way to guard access.

jemfinch 5y ago
They don't, but C++ programmers especially are familiar with not letting inappropriate things escape scopes; they're much less familiar with not letting inappropriate things escape _expressions_. I've reviewed over a half million lines of code from engineers of every level of experience, and I've seen far more bugs from the latter than the former. For instance, a former employer had a function similar to folly::get_default, except it returned the default by reference, and engineers were always running afoul of lifetimes using that function. (It was especially egregious with std::string temporaries returned from string literal defaults.) Thread annotations also effectively prevent unsynchronized access to the object, but they do it in a way that programmers make fewer mistakes with, and they do it with a less error prone mental model: mutexes are seen as guards of _class invariants_, not merely as guards of _data members_. Apart from encouraging a superior mental model, thread annotations also encourage a one-mutex-per-class model which I've found far more successful and error prone (both in code I've reviewed and in code I've written) over a one-mutex-per-member model which inevitably (in my experience) results in fewer bugs.