Closed
Bug 414764
Opened 17 years ago
Closed 17 years ago
CToken freed too many times with <table>m</p>
Categories
(Core :: DOM: HTML Parser, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9beta4
People
(Reporter: martijn.martijn, Assigned: mrbkap)
References
Details
(Keywords: testcase)
Attachments
(1 file)
|
544 bytes,
patch
|
jst
:
review+
jst
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
I get this output with current debug trunk build and trace-malloc enabled:
== BloatView: ALL (cumulative) LEAK STATISTICS
|<----------------Class--------------->|<-----Bytes------>|<----------------Objects---------------->|<--------------References-------------->|
Per-Inst Leaked Total Rem Mean StdDev Total Rem Mean StdDev
0 TOTAL 23 0 503690 0 ( 1065.91 +/- 1396.41) 1065960 -1 ( 1365.49 +/- 2149.51)
15 CToken 24 0 770 0 ( 150.72 +/- 110.86) 1247 -1 ( 148.69 +/- 108.09)
I'm a bit confused by this. CToken is mentioned, but it doesn't seem to have been leaking, but why is it then shown? Or is this still a memory leak?
|
Assignee | |
Comment 1•17 years ago
|
||
Oops, this is a bug caused by the fix for bug 142965. It's the opposite of a leak, we're freeing a token one too many times.
Blocks: 142965
|
|
Assignee | |
Comment 2•17 years ago
|
||
I can't get enough context in the diff (even with -U8) but if you look at the file, you'll see a corresponding IF_HOLD in the other case.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #300280 -
Flags: superreview?(jst)
Attachment #300280 -
Flags: review?(jst)
|
|
Assignee | |
Comment 3•17 years ago
|
||
I doubt this is exploitable, but we should probably fix it.
Flags: blocking1.9?
Priority: -- → P2
Target Milestone: --- → mozilla1.9beta4
|
||
Updated•17 years ago
|
Keywords: testcase
Summary: Memory leak with <table>m</p>? → CToken freed too many times with <table>m</p>
|
||
Updated•17 years ago
|
Attachment #300280 -
Flags: superreview?(jst)
Attachment #300280 -
Flags: superreview+
Attachment #300280 -
Flags: review?(jst)
Attachment #300280 -
Flags: review+
|
|
Assignee | |
Comment 4•17 years ago
|
||
Comment on attachment 300280 [details] [diff] [review]
Fix
This doesn't need to go in for b3, but it's a pretty safe code correctness fix.
Attachment #300280 -
Flags: approval1.9?
Sounds scary as these thing generally are exploitable. So unless we're 100% sure this one is not we should really fix it.
Flags: blocking1.9? → blocking1.9+
|
||
Comment 6•17 years ago
|
||
Comment on attachment 300280 [details] [diff] [review]
Fix
When in doubt with mem - fix it
Attachment #300280 -
Flags: approval1.9? → approval1.9+
|
|
||
Comment 7•17 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•