nsHTTPIndex::CommonInit leaks RDFServiceImpl

VERIFIED FIXED

Status

P3
normal
VERIFIED FIXED
18 years ago
14 years ago

People

(Reporter: dbaron, Assigned: mozilla)

Tracking

({memory-leak})

Trunk
memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tind-mlk])

Attachments

(4 attachments)

nsHTTPIndex::CommonInit leaks references to the RDFService when called multiple
times, since it does a GetService into a global that can already contain an
owning reference.  This leak was introduced in revision 1.51 on August 14 to fix
bug 47927.
If you don't want to do any other cleanup of |Init| and |CommonInit|, and you
think that patch is right, feel free to review the patch and assign the bug to
me...
QA Contact: sairuh → tever
(Assignee)

Comment 3

18 years ago
Created attachment 18490 [details] [diff] [review]
diff for nsDirectoryViewer.cpp
(Assignee)

Comment 4

18 years ago
Created attachment 18491 [details] [diff] [review]
diff for nsDirectoryViewer.h
(Assignee)

Comment 5

18 years ago
Due to the nature of nsDirectoryViewer where it can act both as a singleton and 
not, the RDF service variable shouldn't be a shared global; instead, make it a 
method variable.

dbaron, care to try out the attached diff(s) ?
Status: NEW → ASSIGNED
(Assignee)

Comment 6

18 years ago
Created attachment 18492 [details] [diff] [review]
diff #2 for nsDirectoryViewer
(Assignee)

Comment 7

18 years ago
Oops, no need for nsServiceManager::ReleaseService() when using do_GetService... 
so use second diff for nsDirectoryViewer.cpp
Patch works fine -- doesn't leak.  A few comments:

 * weird whitespace stuff going on.  Looks like tabs->spaces in some places and
spaces->tabs in others.  I guess it should probably be tabs->spaces (but you
don't need to hit stuff you didn't change otherwise).

 * No need to null out the nsCOMPtr in the destructor - it will be freed at the
end of the dtor anyway.

 * You can use do_GetService(..., &rv) to propagate the return value the way you
did before, rather than make up NS_ERROR_NULL_POINTER.

With that fixed, r=dbaron if you want it.
(Assignee)

Comment 9

18 years ago
Chris (Waterson), more r= bug love needed.  :)

Comment 10

18 years ago
[r,sr]=waterson, pick one!
(Assignee)

Comment 11

18 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED

Updated

18 years ago
Status: RESOLVED → VERIFIED

Comment 12

18 years ago
verified
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.