The patch is wrong, nsCycleCollectionISupports doesn't support AddRef/Release.
On 02/04/15 19:58, Piotr Caban wrote:
The patch is wrong, nsCycleCollectionISupports doesn't support AddRef/Release.
But "*ppv·=·&This->IHTMLDOMNode_iface" does. Without an AddRef it causes an imbalance of AddRef/Release.
So, should it be returning IHTMLDOMNode_iface then?
Best Regards Alistair Leslie-Hughes.
On 04/02/15 11:16, Alistair Leslie-Hughes wrote:
But "*ppv·=·&This->IHTMLDOMNode_iface" does. Without an AddRef it causes an imbalance of AddRef/Release.
So, should it be returning IHTMLDOMNode_iface then?
I'm not sure if I understand this code correctly but I think that nsCycleCollectionISupports is an empty class that only needs a pointer. There's no imbalance of AddRef/Release because Release will be never called in this case.
On 02.04.2015 12:23, Piotr Caban wrote:
On 04/02/15 11:16, Alistair Leslie-Hughes wrote:
But "*ppv·=·&This->IHTMLDOMNode_iface" does. Without an AddRef it causes an imbalance of AddRef/Release.
So, should it be returning IHTMLDOMNode_iface then?
I'm not sure if I understand this code correctly but I think that nsCycleCollectionISupports is an empty class that only needs a pointer. There's no imbalance of AddRef/Release because Release will be never called in this case.
I think it's just a marker, so gecko knows that this node is included in collection business.
On 02/04/15 11:26, Nikolay Sivov wrote:
On 02.04.2015 12:23, Piotr Caban wrote:
On 04/02/15 11:16, Alistair Leslie-Hughes wrote:
But "*ppv·=·&This->IHTMLDOMNode_iface" does. Without an AddRef it causes an imbalance of AddRef/Release.
So, should it be returning IHTMLDOMNode_iface then?
I'm not sure if I understand this code correctly but I think that nsCycleCollectionISupports is an empty class that only needs a pointer. There's no imbalance of AddRef/Release because Release will be never called in this case.
I think it's just a marker, so gecko knows that this node is included in collection business.
Yes, it is a special case, see [1]. It's used to cannonicalize interface pointer so that it may be used as an identifier in cycle collector graph. It's not a real interface and it should not cause reference change.
Alistair, your bug is probably somewhere else. If your patch "fixed" it, it indeed suggests a refcount-related problem somewhere else.
Cheers, Jacek
[1] http://hg.mozilla.org/mozilla-central/file/944fb6e682b8/xpcom/glue/nsCycleCo...
Hi Jaceb,
I think it's just a marker, so gecko knows that this node is included in collection business.
Yes, it is a special case, see [1]. It's used to cannonicalize interface pointer so that it may be used as an identifier in cycle collector graph. It's not a real interface and it should not cause reference change.
Alistair, your bug is probably somewhere else. If your patch "fixed" it, it indeed suggests a refcount-related problem somewhere else.
Yes, the patch appeared to "fix" the issue. I'll see if I can track the problem down.
Best Regards Alistair Leslie-Hughes