Universalchardet leaks memory

VERIFIED FIXED in mozilla1.0

Status

()

P3
major
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: ttathome, Assigned: shanjian)

Tracking

({intl, memory-leak})

Trunk
mozilla1.0
intl, memory-leak
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
The universal_charset_detector always leaks memory at its deallocation due to
the broken destructor chain.
I happened to notice the fact while running a standalone program which processed
a large set of files via the component in a one instance per file manner.
(The program essentially is an infinite loop version of the UniversalChardetTest.)

The offending class is nsCharSetProber, the base class of many other Prober
classes, and the missing virtual destructor in the class prevents the invocation
of the derived classes' destructors.

The patch will follow.
(Reporter)

Comment 1

17 years ago
Created attachment 66671 [details] [diff] [review]
Fix for memleaks in the universalchardet module

The fix for the nsCharSetProber class declaration, plus fix for
nsUniversalDetector methods, which may cause another leak when the object is
reused after Reset().
Severity: normal → major
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: mlk, patch, review

Updated

17 years ago
Keywords: intl
QA Contact: ruixu → ylong

Comment 2

17 years ago
pass to shanjian
Assignee: yokoyama → shanjian
Keywords: nsbeta1
Priority: -- → P3
Target Milestone: --- → mozilla1.0
(Assignee)

Comment 3

17 years ago
Comment on attachment 66671 [details] [diff] [review]
Fix for memleaks in the universalchardet module

r=shanjian
Attachment #66671 - Flags: review+
(Assignee)

Comment 4

17 years ago
brendan, could you sr? 
Status: NEW → ASSIGNED
Comment on attachment 66671 [details] [diff] [review]
Fix for memleaks in the universalchardet module

sr=brendan@mozilla.org

/be
Attachment #66671 - Flags: superreview+
This patch needs to be checked in as soon as possible.
it is a no-brainer, can we get this for 0.9.9 ?
(Assignee)

Comment 7

17 years ago
Because there are too many checkin yesterday, all checkins have to be metered.
Unfortunately, I did not got the permission to check in this patch. I will try
it for 1.0. 

Comment 8

17 years ago
nsbeta1+, shanjian- do we have the same problem in the all detector. if so,
please open a bugscape bug and fix that (mark nsbeta1)
Keywords: nsbeta1 → nsbeta1+
Shanjian, just check it in when the tree opens later today for approved 0.9.9
checkins (it's now closed for smoketest blocker fixing only),
a=brendan@mozilla.org on behalf of drivers.

/be
(Assignee)

Comment 10

17 years ago
fix checked in. 
(Assignee)

Comment 11

17 years ago
marked as fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 12

17 years ago
Seems works fine with me on latast trunk build, mark as verified.  Please
re-open if still see the problem.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.