Re: mshtml: Removed return to ensure AddRef is called on an internal interface
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.
Created a bug. https://bugs.winehq.org/show_bug.cgi?id=38340 Best Regards Alistair Leslie-Hughes
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
participants (4)
-
Alistair Leslie-Hughes -
Jacek Caban -
Nikolay Sivov -
Piotr Caban