Private members of ref counted classes and lambdas

[Andrew Osmond]

Suppose I have some ref counted class Foo with the private member mBar.
Normally with a lambda expression, you can easily access the private
members by passing the this pointer:

void Foo::pokeBar()
{
nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction[this] () -> void
{
mBar.poke();
});
NS_DispatchToMainThread(r);
}

But obviously the Foo object could get released before the dispatch
completes. I could capture an nsRefPtr to this in the lambda:

nsresult Foo::pokeBar()
{
nsRefPtr<Foo> self = this;
nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction[self] () -> void
{
self->mBar.poke();
});
NS_DispatchToMainThread(r);
}

But now it won't let me access the private members. I can add *both* and it
appears to work (at least on gcc, haven't tested clang/msvc):

nsresult Foo::pokeBar()
{
nsRefPtr<Foo> self = this;
nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction[this, self] () -> void
{
self->mBar.poke();
});
NS_DispatchToMainThread(r);
}

Since I'm using the self pointer, I don't see the compiler optimizing the
capture of it out, and the addition of "this" opens up the scope to what I
need. (Although maybe explicitly using self is overkill? Can I eliminate
that safely?)

But this is my first time using lambda expressions in C++, so I'm not
entirely sure if there is a good reason for *not* doing what I am above :).
In the few examples of lambdas that I found in our code, it looks like we
just made everything public. My other attempts at fixing this was to make
an nsRefPtr to mBar inside Foo::pokeBar and pass *that* to the lambda,
which is fine but my real world uses have multiple private members and
functions I would like to access :). Is there anybody who can chime in on
how best to use lamdas with private members?

The only guidance I could find on the wiki was this, which I think I am
following (e.g. don't use [=] to suck down everything):
https://developer.mozilla.org/en-US/docs/Mozilla/C++_Portability_Guide#Use_C.2B.2B_lambdas.2C_but_with_care

Thanks,


Andrew

[Daniel Holbert]

On 06/04/2015 07:29 AM, Andrew Osmond wrote:
> Suppose I have some ref counted class Foo with the private member mBar.

> Normally with a lambda expression, [...]

> obviously the Foo object could get released before the dispatch
> completes

You may be interested in this thread from a few months back:
"Proposal to ban the usage of refcounted objects inside C++ lambdas in
Gecko"
https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ

(Not sure it arrived at a concrete conclusion, but you may run across
some pitfalls/suggestions at least. I haven't used lambdas in C++, so I
won't attempt to directly answer your question.)

~Daniel

[Seth Fowler]

> On Jun 4, 2015, at 11:17 AM, Daniel Holbert <dhol...@mozilla.com> wrote:
>
> You may be interested in this thread from a few months back:
> "Proposal to ban the usage of refcounted objects inside C++ lambdas in
> Gecko"
> https://groups.google.com/d/msg/mozilla.dev.platform/Ec2y6BWKrbM/xpHLGwJ337wJ
>
> (Not sure it arrived at a concrete conclusion, but you may run across
> some pitfalls/suggestions at least. I haven't used lambdas in C++, so I
> won't attempt to directly answer your question.)

My impression was that the conclusion was “refcounted objects are not banned inside C++ lambdas” - i.e., no policy change from the status quo - "but we need to be aware of the pitfalls”. The pitfalls are discussed pretty thoroughly in the thread.

- Seth

[Andrew Osmond]

Turns out my original problem was some other mistake I made. Using just
self works (thanks botond for the poke on IRC about that).

I remember reading the linked thread, although I had since forgotten about
it -- thanks for the reminder. My impression was that using raw pointers
for ref counted objects with lambdas is something to be discouraged,
rightfully so. That's why I use nsRefPtr, just like I would if I explicitly
used a nested class inheriting from nsRunnable instead of a lambda. I'll
review the thread again though (I *did* forgot about using Move, perhaps
other bits as well).

[Botond Ballo]

On Thu, Jun 4, 2015 at 2:24 PM, Seth Fowler <se...@mozilla.com> wrote:
> My impression was that the conclusion was “refcounted objects are not banned inside C++ lambdas” - i.e., no policy change from the status quo - "but we need to be aware of the pitfalls”. The pitfalls are discussed pretty thoroughly in the thread.

Capturing of raw pointers to refcounted objects was in fact banned - a
static analysis forbidding it was introduced in bug 1153304.

(Note that, as I discovered recently, the static analysis was
over-eager and was also complaining about raw pointers to refcounted
object being taken as an argument or used as a local variable inside a
lambda - I'm about to land a fix for that in bug 1170388).

Cheers,
Botond