Closed Bug 345277 Opened 18 years ago Closed 18 years ago

When "Certificate Viewer" is shut, the object leaks

Categories

(Core :: Security: PSM, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: sugar.waffle, Assigned: KaiE)

References

()

Details

(Keywords: memory-leak, verified1.8.1.2)

Attachments

(1 file)

Reproducible: Always

Steps to Reproduce:
1. Open URL
2. Open Tool --> Page Info
3. Click Security tab
4. Click View button
5. Click Close button on Certifitate Viewer

Leak Monitor report:

Leaks in window 0x2498460:
[ ] [leaked object] (2613a18) = [object Object]
[ ] [leaked object] (2613a18) = [object Object]

Windows XP SP1
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060719 Minefield/3.0a1
I just stumbled this too.  
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060904 Minefield/3.0a1 ID:2006090404 [cairo]
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: mlk
I think it's leaking its listener callback object somehow. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/pki/resources/content/viewCertDetails.js&rev=1.29#195
Assignee: nobody → kengert
Component: General → Security: PSM
OS: Windows XP → All
Product: Firefox → Core
QA Contact: general
Hardware: PC → All
Good catch!
I think nsBaseVerificationJob needs a virtual destructor.
Attached patch Patch v1Splinter Review
I confirm, with our existing code, the destructors of our cert verification jobs never get called, resulting in leaks!!!

Adding a virtual constructor to the common base class of all such jobs is sufficient.

In my testing, this caused the destructors of the derived classes to get called, at the time the verification thread deletes the job via its base pointer.
Attachment #240261 - Flags: review?(rrelyea)
*** Bug 354507 has been marked as a duplicate of this bug. ***
*** Bug 354646 has been marked as a duplicate of this bug. ***
Kaie, what was the original class that was leaking?

bob
(In reply to comment #7)
> Kaie, what was the original class that was leaking?

The classes that leak are the ones derived from the base nsBaseVerificationJob:
- nsCertVerificationJob
- nsSMimeVerificationJob

In order to clean up correctly, the destructors of those function must get called. But currently they never get called. Because we delete the base class pointer.
Comment on attachment 240261 [details] [diff] [review]
Patch v1

r+= rrelyea
Attachment #240261 - Flags: review?(rrelyea) → review+
fixed on trunk
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Comment on attachment 240261 [details] [diff] [review]
Patch v1

I propose to check in this correctness fix to the 1.8 branch.
Attachment #240261 - Flags: approval1.8.1.2?
Attachment #240261 - Flags: approval1.8.1.1?
*** Bug 363369 has been marked as a duplicate of this bug. ***
(In reply to comment #12)
> *** Bug 363369 has been marked as a duplicate of this bug. ***
> 

hi, 
when will we (end users) see this fix? just asking for a time+version estimate -thanks
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.2?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.2?
Flags: blocking1.8.1.2+
Comment on attachment 240261 [details] [diff] [review]
Patch v1

Approved for 1.8 branch, a=jay for drivers.
Attachment #240261 - Flags: approval1.8.1.2?
Attachment #240261 - Flags: approval1.8.1.2+
Attachment #240261 - Flags: approval1.8.1.1?
Checked in to 1.8 branch.

Checking in nsVerificationJob.h;
/cvsroot/mozilla/security/manager/ssl/src/nsVerificationJob.h,v  <--  nsVerificationJob.h
new revision: 1.1.2.3; previous revision: 1.1.2.2
done
Keywords: fixed1.8.1.2
Verified Fixed for 1.8.1.2 with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/2007011004 BonEcho/2.0.0.2pre on Windows x64 and on Linux Fedora FC6
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: