Closed
Bug 338677
Opened 18 years ago
Closed 18 years ago
nsUrlClassifierDBService leaks memory
Categories
(Toolkit :: Safe Browsing, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tony, Assigned: tony)
References
Details
(Keywords: fixed1.8.1)
Attachments
(2 files, 2 obsolete files)
7.25 KB,
patch
|
darin.moz
:
review+
darin.moz
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
4.45 KB,
patch
|
Details | Diff | Splinter Review |
mConnection and gEventQ are regular pointers so they need manual release. We also don't release the reference to the thread queue from mWorker when we're done with it.
Assignee | ||
Comment 1•18 years ago
|
||
This is for branch; I'm working on a patch for trunk.
Attachment #222744 -
Flags: review?(darin)
Assignee | ||
Comment 2•18 years ago
|
||
Attachment #222756 -
Flags: review?(darin)
Comment 3•18 years ago
|
||
Comment on attachment 222756 [details] [diff] [review] v1: patch for trunk >Index: nsUrlClassifierDBService.cpp > nsUrlClassifierDBServiceWorker::CloseDb() > { > if (mConnection != nsnull) { >+ NS_RELEASE(mConnection); > delete mConnection; > mConnection = nsnull; So, NS_RELEASE(x) will null out |x| as a side-effect. That means that |delete x| and |x = nsnull| will both be no-ops, so they should be removed. Besides, I don't think you meant to delete this object! :) > gDbBackgroundThread->Shutdown(); > NS_RELEASE(gDbBackgroundThread); >+ gDbBackgroundThread = nsnull; No need for explicit nulling here. r=darin with those nits picked
Attachment #222756 -
Flags: review?(darin) → review+
Assignee | ||
Comment 4•18 years ago
|
||
remove extra delete and null
Attachment #222744 -
Attachment is obsolete: true
Attachment #222744 -
Flags: review?(darin)
Updated•18 years ago
|
Attachment #222906 -
Flags: review+
Attachment #222906 -
Flags: approval-branch-1.8.1+
Assignee | ||
Comment 5•18 years ago
|
||
extra null assignment removed
Attachment #222756 -
Attachment is obsolete: true
Comment 6•18 years ago
|
||
fixed-on-trunk, fixed1.8.1
Comment 7•18 years ago
|
||
I think the trunk patch doesn't do the right thing: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp&mark=460,461&rev=1.17#460
Updated•10 years ago
|
Product: Firefox → Toolkit
You need to log in
before you can comment on or make changes to this bug.
Description
•