434074 - TestRegistrationOrder test leaks stuff

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jeff Walden [:Waldo]]

Attachment: A few NS_RELEASES for pointers that must be weak, smart pointers for the rest

It's test output clutter, and if we could figure out a way to do it cleanly, it'd be good to be able to use C++ unit tests to ensure we don't regress and start leaking in particular cases.  Not sure how that would work yet, but it's certainly not possible without this patch.

No-leak behavior can be verified with this command:

XPCOM_MEM_LEAK_LOG=1 XPCOM_DEBUG_BREAK=stack-and-abort \
  ./run-mozilla.sh ./TestRegistrationOrder $topsrcdir/xpcom/tests/regorder/

I'll probably try to grab someone on IRC either for a review or a rubber-stamp, not worth spending too much time on this.

Comment 1

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

Comment on attachment 321300 [details] [diff] [review]
A few NS_RELEASES for pointers that must be weak, smart pointers for the rest

>+  // Using an extra scope here so that dirSvcProvider will acquire its first
>+  // AddRef after XPCOM startup, so XPCOM_MEM_LEAK_LOG won't lie and say
>+  // that it leaked (after this block, dirSvcProvider dangles).
>+  {

It doesn't seem to me like this scope changes anything, since it ends at the end of the function anyway.

Also, why initialize rv = 0 twice?

Not sure why the three service pointers can't be nsCOMPtr<nsISupports> with do_CreateInstance...

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Smart pointers for all!

(In reply to comment #1)
> It doesn't seem to me like this scope changes anything, since it ends at the
> end of the function anyway.

Hrm, this was only necessary because, at the time, I was addrefing it after construction, which meant the leak detection code wasn't seeing the first addref and thought it was leaking.

> Not sure why the three service pointers can't be nsCOMPtr<nsISupports> with
> do_CreateInstance...

Mostly because I didn't want to change test code where I wasn't sure it wasn't doing so for some particular reason, but it seems there wasn't really a reason to worry about that at a closer look.

Comment 3

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

Comment on attachment 321308 [details] [diff] [review]
Smart pointers for all!

r=dbaron

Comment 4

[Jeff Walden [:Waldo]]

Fixt in mozilla-central.