Closed
Bug 391491
Opened 17 years ago
Closed 17 years ago
Windows trace-malloc needs to override new[] and delete[]
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: dbaron, Assigned: dbaron)
Details
Attachments
(2 files, 1 obsolete file)
1.85 KB,
patch
|
brendan
:
review+
brendan
:
review+
|
Details | Diff | Splinter Review |
14.13 KB,
patch
|
brendan
:
review+
brendan
:
review+
|
Details | Diff | Splinter Review |
With Windows trace-malloc, I'm currently seeing allocations done with new[] all showing up as leaked, since new[] seems to call through to new, but delete[] doesn't call through to delete. Or something like that. Maybe it has to with offsetting pointers for the stored object count (which is needed to call the right destructors) at different places. Overriding both new[] and delete[] seems to fix the log of shutdown leaks, but I'm not sure it's the right thing to do. I need to investigate further tomorrow.
Assignee | ||
Comment 1•17 years ago
|
||
Assignee | ||
Comment 2•17 years ago
|
||
Yeah, that fixes --shutdown-leaks but makes leakstats on the log generated by --trace-malloc give a negative number of leaked allocations. So it's making is double-count somewhere...
Assignee | ||
Comment 3•17 years ago
|
||
That patch had a typo; I'll test if this one is better tomorrow, or if it needs tweaks for underlying asymmetry.
Attachment #275935 -
Attachment is obsolete: true
Assignee | ||
Comment 4•17 years ago
|
||
This adds suppression around the calls to the original allocation functions in case they call each other. It doesn't fix the negative allocation count I was seeing, though (perhaps due to not starting up until we're into main), but I think it's probably necessary to avoid some errors in the logs.
Attachment #275995 -
Flags: review?(brendan)
Assignee | ||
Updated•17 years ago
|
Attachment #275946 -
Flags: review?(brendan)
Comment 5•17 years ago
|
||
Comment on attachment 275946 [details] [diff] [review] patch to override both new[] and delete[] r=me, looks right, if it works, commit at will! /be
Attachment #275946 -
Flags: review?(brendan)
Attachment #275946 -
Flags: review+
Comment 6•17 years ago
|
||
Comment on attachment 275995 [details] [diff] [review] add suppression around calls to original functions get_tm_thread might want to be tm_get_thread or some such, just to follow some anal-retentive naming convention that wards off name collision in my dreams. /be
Attachment #275995 -
Flags: review?(brendan)
Attachment #275995 -
Flags: review+
Assignee | ||
Comment 7•17 years ago
|
||
Patches checked in to trunk, 2007-08-10 15:21 -0700 and 15:22 -0700.
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
•