56744 - CompositeDataSourceImpl::Release doesn't always work

Status: VERIFIED FIXED

(Core Graveyard :: RDF, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

If I run ./mozilla -ProfileManager and click Exit, 2 of the 10-11 nsVoidArray
leaked (the first two by serial number) are the nsVoidArray that are part of a
CompositeDataSourceImpl.  This CompositeDataSourceImpl doesn't show up in the
leak stats, but it wasn't deleted because CompositeDataSourceImpl::Release
doesn't always work.  In particular, if mDataSources.Count() == 0, it doesn't
|delete this|.

The proposed fix I'll attach is just a slightly modified version of my patch to
bug 44621.  waterson:  What do you think?

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: proposed fix

Comment 2

[Chris Waterson]

looks good; r=waterson

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: better fix - don't confuse the refcount logging -- always NS_LOG_RELEASE(...0...) to indicate destruction

Comment 4

[Chris Waterson]

r=waterson, for real this time!

Comment 5

[Scott Collins]

would prefer |NS_STATIC_CAST(PRInt32*, &mRefCnt)| in the two places where you
use an old-style cast

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

When I change it to NS_STATIC_CAST(PRInt32 *, ...), I get:

/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp: In method
`nsrefcnt
CompositeDataSourceImpl::Release ()':
/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp:572: invalid
static_cast from type
`nsrefcnt *' to type `PRInt32 *'
/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp:586: invalid
static_cast from type
`nsrefcnt *' to type `PRInt32 *'
gmake[2]: *** [nsCompositeDataSource.o] Error 1

If I make the static cast to PRUint32 (which is what nsrefcnt is already, so
it's kinda pointless), I get:

/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp: In method
`nsrefcnt
CompositeDataSourceImpl::Release ()':
/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp:572: cannot
convert `PRUint32 *' to
`PRInt32 *' for argument `1' to `PR_AtomicDecrement (PRInt32 *)'
/builds/seamonkey/mozilla/rdf/base/src/nsCompositeDataSource.cpp:586: cannot
convert `PRUint32 *' to
`PRInt32 *' for argument `1' to `PR_AtomicAdd (PRInt32 *, int)'
gmake[2]: *** [nsCompositeDataSource.o] Error 1

I'm using RedHat's gcc "2.96".  Any better ideas, or should I just leave it as
it was?

Comment 7

[Chris Waterson]

scc, did you mean NS_REINTERPRET_CAST? I thought that's what you needed to go
between unrelated types...

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch with NS_REINTERPRET_CAST

Comment 9

[Scott Collins]

Right :-)  even though signed and unsigned are convertable to each other,
pointers to the two types are unrelated, and a reinterpret cast is required.
|NS_STATIC_CAST| was the wrong thing for me to say, sorry.

sr=scc

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2000-10-29 13:32-0800.

Comment 11

[Tom Everingham]

verified