TestRegistrationOrder test leaks stuff

RESOLVED FIXED in mozilla2.0

Status

()

Core
XPCOM
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

({memory-leak})

Trunk
mozilla2.0
memory-leak
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 321300 [details] [diff] [review]
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 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...
(Assignee)

Comment 2

10 years ago
Created attachment 321308 [details] [diff] [review]
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.
Attachment #321300 - Attachment is obsolete: true
Comment on attachment 321308 [details] [diff] [review]
Smart pointers for all!

r=dbaron
Attachment #321308 - Flags: review+
(Assignee)

Comment 4

10 years ago
Fixt in mozilla-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0
You need to log in before you can comment on or make changes to this bug.