On 4/10/15 2:09 PM, Seth Fowler wrote:
On Apr 10, 2015, at 8:46 AM, Ehsan Akhgari <ehsan.akhg...@gmail.com> 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.
The above is a raw pointer bug, not a lambda bug. The above wo/lambdas:
class MyRunnable
{
public:
MyRunnable(nsINode* aMyNode) : mMyNode(aMyNode) {}
void Run() { myNode->Foo(); }
private:
nsINode* mMyNode;
};
nsINode* myNode;
TakeFunc(new MyRunnable(myNode));
That's just as bad, and harder to spot! [1]
IMHO the use of lambdas helps spot the problem, by
1. Being more precise (less boilerplate junk for bugs to hide in), and
2. lambda capture use safer copy construction by default (hence the
standout [&] above for reviewers).
> Lambdas will be much less useful if they can’t capture refcounted
objects, so I’m strongly against banning that.
+1.
Case in point, we use raw pointers with most runnables, a practice
established in NS_DispatchToMainThread [2]. Look in mxr/dxr for the 100+
uses of NS_DispatchToMainThread(new SomeRunnable()).
The new ban would prevent us from passing runnables to lambdas, like [3]
MyRunnableBackToMain* runnable = new MyRunnableBackToMain();
nsRefPtr<media::ChildPledge<nsCString>> p = SomeAsyncFunc();
p->Then([runnable](nsCString result) mutable {
runnable->mResult = result;
NS_DispatchToMainThread(runnable);
});
So I think this ban is misguided. These are old sins not new to lambdas.
- Seth
.: Jan-Ivar :.
[1] Bonus if you caught that new MyRunnable() is a raw pointer as well.
[2]
http://mxr.mozilla.org/mozilla-central/source/xpco/glue/nsThreadUtils.cpp?mark=164-168#164
[3]
http://mxr.mozilla.org/mozilla-central/source/dom/media/MediaManager.cpp?mark=1516-1521#1501
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
