Re: refcounting - which types to use ?

[Boris Zbarsky]

On 8/16/17 6:56 AM, Enrico Weigelt, metux IT consult wrote:

I'd assume that do_CreateInstance() returns the object w/ refcnt=1.


Yes.  It returns it as an already_AddRefed<> struct.


The assignment to nsCOMPtr should increase refcount. Correct ?


No, because nsCOMPtr knows not to increase the refcount when an 
already_AddRefed is assigned to it.  If it _did_ increase it, that would 
be a leak.



When the function is left, the nsCOMPtr is destroyed, and refcnt
goes back to 1 (we now have a reference left in the caller's pointer
field).


The refcount goes to 0 and the object is destroyed.  You want to either 
list.forget(_retval) or NS_ADDREF(*_retval = list), with the former much 
preferred.



I'd guess getter_AddRefs() causes refcnt to be increased again


No.  getter_AddRefs just nulls out the nsCOMPtr (causing it to release 
whatever it's holding right now, if anything) so the direct assignment 
into the nsCOMPtr's guts will not leak whatever used to be there.  It 
also makes it clear that there is a reference handoff here: after the 
call, the nsCOMPtr is holding a ref, but that ref was originally 
incremented inside the getter function being called.



So, should I use dont_AddRef() here ?


No.


Is the situation different when using out parameter instead of rval ?


No.


BTW: what happens when passing nsCOMPtr as a reference (w/o IDL) ?


Nothing special.  The code you suggest would work; it's just a bit hard 
to reason about at the callsite.



I'd assume that the last line will fill the caller's ref field w/ the
pointer and increase the object's refcnt (so it's 2 now), then when
callee is left, its myref is destroyed and refcnt is back to 1.

Is that correct ?


Yes.

-Boris

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

Re: refcounting - which types to use ?

[Ehsan Akhgari]

On 08/24/2017 06:04 AM, jyaven...@mozilla.com wrote:

On Thursday, August 24, 2017 at 11:56:53 AM UTC+2, James Cheng wrote:

How could I make it compile error?

I did write

RefPtr xxx = yyy;
xxx.forget();

You need to run the static analyser.

The documentation about MOZ_MUST_USE_TYPE is explained there 
http://searchfox.org/mozilla-central/source/mfbt/Attributes.h#519-521.

This is why you should always do a try run, because the static analyser *will* 
choke on this code.
You can also run the static analyzer locally by using a clang binary 
downloaded from llvm.org, and adding the following line to your 
mozconfig file:


ac_add_options --enable-clang-plugin

We will (hopefully sometime soon) make this extremely easy to do on all 
platforms without needing to download a custom clang or change anything 
in your mozconfig through a new mach command when 
https://bugzilla.mozilla.org/show_bug.cgi?id=1328454 is finally fixed.


Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: refcounting - which types to use ?

[jyavenard]

On Thursday, August 24, 2017 at 11:56:53 AM UTC+2, James Cheng wrote:
> How could I make it compile error?
> 
> I did write
> 
> RefPtr xxx = yyy;
> xxx.forget();

You need to run the static analyser.

The documentation about MOZ_MUST_USE_TYPE is explained there 
http://searchfox.org/mozilla-central/source/mfbt/Attributes.h#519-521.

This is why you should always do a try run, because the static analyser *will* 
choke on this code.

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

Re: refcounting - which types to use ?

[James Cheng]

How could I make it compile error?

I did write

RefPtr xxx = yyy;
xxx.forget();

It is ok for compiling on my Mac.

The comment [1] said " Makes it a compile time error to not use the return
value of a function which has this type", but how do I make the clang
format take effect?

BTW, if you want to ignore the return value, you can use the special
version of Unused [2], but it will leak if you don't manually delete the
pointer.
[1]
http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/mfbt/Attributes.h#519-521
[2]
http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/mfbt/AlreadyAddRefed.h#112-117

2017-08-24 17:31 GMT+08:00 :

> On Thursday, August 17, 2017 at 1:02:56 PM UTC+2, Aryeh Gregor wrote:
> > Note that in this case you cannot ignore the return value -- it must
> > be assigned to an nsCOMPtr or a RefPtr, or else it will leak.  Ideally
> > we would allow a return type of nsCOMPtr&&, but
> > currently I think we don't.
>
> This is not entirely correct.
>
> alreadyAddRefed is marked MOZ_MUST_USE_TYPE.
>
> If you do not use the return value, it will not compile.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: refcounting - which types to use ?

[jyavenard]

On Thursday, August 17, 2017 at 1:02:56 PM UTC+2, Aryeh Gregor wrote:
> Note that in this case you cannot ignore the return value -- it must
> be assigned to an nsCOMPtr or a RefPtr, or else it will leak.  Ideally
> we would allow a return type of nsCOMPtr&&, but
> currently I think we don't.

This is not entirely correct.

alreadyAddRefed is marked MOZ_MUST_USE_TYPE.

If you do not use the return value, it will not compile.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: refcounting - which types to use ?

[Aryeh Gregor]

On Thu, Aug 17, 2017 at 7:07 AM, Enrico Weigelt, metux IT consult
 wrote:
> OTOH, is there a way to create versions that really return it as rval,
> so I conveniently could just call like that ?
>
>nsCOMPtr list = addrbook->FindRecipients(addr);

You can do this with a return type of
already_AddRefed.  Then instead of

  list.forget(_retval);
  return NS_OK;

you do just

  return list.forget();

Note that in this case you cannot ignore the return value -- it must
be assigned to an nsCOMPtr or a RefPtr, or else it will leak.  Ideally
we would allow a return type of nsCOMPtr&&, but
currently I think we don't.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: refcounting - which types to use ?

[Enrico Weigelt, metux IT consult]

On 17.08.2017 04:03, Shih-Chiang Chien wrote:
You should use |forget| to transfer the ownership of the nsIArray from 
list to _retval.


Ok, thanks. Already suspected that (found some similar things in the
code). Could we update the docs (maybe some set of examples) ?

IIRC, there're some places that do it like in my prev mail, which I took
as example (just forget where exactly that was) - should that be fixed ?

By the way: is there a difference between out parameters and rval ?
Or can I just assume, rval's are equivalent to an out parameter ?

OTOH, is there a way to create versions that really return it as rval,
so I conveniently could just call like that ?

   nsCOMPtr list = addrbook->FindRecipients(addr);


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

Re: refcounting - which types to use ?

[Shih-Chiang Chien]

You should use |forget| to transfer the ownership of the nsIArray from list
to _retval.

/* nsIArray FindRecipients (in AString name); */
NS_IMETHODIMP nsAddrBookService::FindRecipients(const nsAString & addr,
nsIArray * *_retval)
{
nsresult rv = NS_OK;
nsCOMPtr list =
do_CreateInstance(NS_ARRAY_CONTRACTID, );
NS_ENSURE_SUCCESS(rv, rv);

if (NS_FAILED(rv = FillRecipients(addr, list)) {
  return rv;
}

list.forget(_retval);
return NS_OK;
}

On Wed, Aug 16, 2017 at 6:56 PM, Enrico Weigelt, metux IT consult <
enrico.weig...@gr13.net> wrote:

> Hi folks,
>
>
> I'm still a bit confused on which types (nsCOMPtr, ...) to use when
> exactly, in combination w/ IDLs.
>
> Let's consider a case where an nsArray is created and returned
> (as rval, not out-parameter):
>
> IDL:
>
> [scriptable, uuid(ea0d8b3d-a549-4666-82d8-3a82cee2a3f1)]
> interface nsIAddrBookService : nsISupports
> {
> nsIArray FindRecipients(in AString name);
> ...
> }
>
> C++:
>
> /* nsIArray FindRecipients (in AString name); */
> NS_IMETHODIMP nsAddrBookService::FindRecipients(const nsAString & addr,
> nsIArray * *_retval)
> {
> nsresult rv = NS_OK;
> nsCOMPtr list =
> do_CreateInstance(NS_ARRAY_CONTRACTID, );
> NS_ENSURE_SUCCESS(rv, rv);
> *_retval = list;
>
return FillRecipients(addr, list);
> }
>

> I'd assume that do_CreateInstance() returns the object w/ refcnt=1.
> The assignment to nsCOMPtr should increase refcount. Correct ?
> When the function is left, the nsCOMPtr is destroyed, and refcnt
> goes back to 1 (we now have a reference left in the caller's pointer
> field).
>
> Now the caller side: (inspired by various examples ...)
>
> nsCOMPtr list;
> rv = addrBookService->FindRecipients(
> splittedRecipients[j].mEmail,
> getter_AddRefs(list));
>
> I'd guess getter_AddRefs() causes refcnt to be increased again, so
> we're back at 2. That could lead to a leak, when that function returns
> (and doesn't pass the ref anywhere else).
>
> So, should I use dont_AddRef() here ?
>
> Is the situation different when using out parameter instead of rval ?


>
> BTW: what happens when passing nsCOMPtr as a reference (w/o IDL) ?
>
> Assume the following code:
>
> void caller() {
> nsCOMPtr ref;
>
> callee(ref);
> ref->Something();
> }
>
> void callee(nsCOMPtr & outref) {
> nsCOMPtr myref = do_CreateInstance(...);
> ...
> outref = myref;
> }
>
> I'd assume that the last line will fill the caller's ref field w/ the
> pointer and increase the object's refcnt (so it's 2 now), then when
> callee is left, its myref is destroyed and refcnt is back to 1.
>
> Is that correct ?
>
>
> --mtx
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

refcounting - which types to use ?

[Enrico Weigelt, metux IT consult]

Hi folks,


I'm still a bit confused on which types (nsCOMPtr, ...) to use when
exactly, in combination w/ IDLs.

Let's consider a case where an nsArray is created and returned
(as rval, not out-parameter):

IDL:

[scriptable, uuid(ea0d8b3d-a549-4666-82d8-3a82cee2a3f1)]
interface nsIAddrBookService : nsISupports
{
nsIArray FindRecipients(in AString name);
...
}

C++:

/* nsIArray FindRecipients (in AString name); */
NS_IMETHODIMP nsAddrBookService::FindRecipients(const nsAString & addr, 
nsIArray * *_retval)

{
nsresult rv = NS_OK;
nsCOMPtr list =
do_CreateInstance(NS_ARRAY_CONTRACTID, );
NS_ENSURE_SUCCESS(rv, rv);
*_retval = list;
return FillRecipients(addr, list);
}

I'd assume that do_CreateInstance() returns the object w/ refcnt=1.
The assignment to nsCOMPtr should increase refcount. Correct ?
When the function is left, the nsCOMPtr is destroyed, and refcnt
goes back to 1 (we now have a reference left in the caller's pointer
field).

Now the caller side: (inspired by various examples ...)

nsCOMPtr list;
rv = addrBookService->FindRecipients(
splittedRecipients[j].mEmail,
getter_AddRefs(list));

I'd guess getter_AddRefs() causes refcnt to be increased again, so
we're back at 2. That could lead to a leak, when that function returns
(and doesn't pass the ref anywhere else).

So, should I use dont_AddRef() here ?

Is the situation different when using out parameter instead of rval ?


BTW: what happens when passing nsCOMPtr as a reference (w/o IDL) ?

Assume the following code:

void caller() {
nsCOMPtr ref;

callee(ref);
ref->Something();
}

void callee(nsCOMPtr & outref) {
nsCOMPtr myref = do_CreateInstance(...);
...
outref = myref;
}

I'd assume that the last line will fill the caller's ref field w/ the
pointer and increase the object's refcnt (so it's 2 now), then when
callee is left, its myref is destroyed and refcnt is back to 1.

Is that correct ?


--mtx

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