On 2015-04-10 9:43 PM, Gregory Szorc wrote:
On Fri, Apr 10, 2015 at 11:46 AM, Ehsan Akhgari <[email protected] <mailto:[email protected]>> wrote:I would like to propose that we should ban the usage of refcounted objects inside lambdas in Gecko. Here is the reason: Consider the following code: nsINode* myNode; TakeLambda([&]() { myNode->Foo(); }); There is nothing that guarantees that the lambda passed to TakeLambda is executed before myNode is destroyed somehow. While it's possible to verify on a case by case basis that this code is indeed safe, I think it's too error prone to keep these invariants over time. Given the fact that if things go wrong we'd deal with a UAF bug which will be sec-critical, I think it's better to be safe than sorry here. If nobody objects, I'll update the style guide in the next few days. If you do object, please make a case for why the above analysis is incorrect and why the usage of refcounted objects in lambdas in general is safe. Why do we merely update the style guide? We have a custom Clang compiler plugin. I think we should have the Clang compiler plugin enforce our style guidelines by refusing to compile code that is against policy. This would enable reviewers to stop focusing on policing the style guide and enable them to focus more on the correctness of code. This will result in faster reviews and/or higher quality code.
Yes, I'm aware of the clang plugin, as I've written most of the analyses in it. ;-) See bug 1153304. But that is orthogonal, since that plugin doesn't cover every single line of code in the code base. Obvious examples are Windows/B2G specific code.
_______________________________________________ dev-platform mailing list [email protected] https://lists.mozilla.org/listinfo/dev-platform
