Closed
Bug 9341
Opened 25 years ago
Closed 25 years ago
Memory allocation and deallocation for char* data is flawed.
Categories
(Core :: XPCOM, defect, P2)
Core
XPCOM
Tracking
()
VERIFIED
FIXED
M15
People
(Reporter: bruce, Assigned: bruce)
References
Details
This is going to cause anyone running Purify a lot of serious pain once necko
builds become the default.
I've broached this subject in the newsgroups, but haven't seen much discussion
of a solution or checkins that appeared to be a solution.
Allocating memory via nsString::ToNewCString() and freeing it via nsCRT::free()
is bad. Freeing it via nsAllocator::free() is bad as well. There are many many
violations of this everywhere in the necko code and the code checked in for
necko intrgration throughout the rest of the tree.
Can we come to a reasonable solution and make sure that all checkins from here
on out are correct at the least?
Comment 1•25 years ago
|
||
Bruce, Thanks for filing a bug on this. Gagan is eventually going to convert
all the nsIURI accessors to use PRUnichar* or nsString& so we'll get to this
eventually.
Updated•25 years ago
|
Assignee: gagan → warren
Comment 2•25 years ago
|
||
I think something should be done sooner, and not by gagan. Why can't we fix
ToNewCString to use the same allocator as nsCRT and nsAllocator, as we discussed
at last week's architecture meeting. That seems like a smaller fix that will
help all code, not just Necko. Warren, are you up to it, or should scc have a
whack (assuming he's not on the road for days)?
/be
Updated•25 years ago
|
Assignee: warren → scc
Comment 3•25 years ago
|
||
Making ToNewCString and nsCRT use the same allocator seems like the right
thing. Scott, are you interested in handling this?
Updated•25 years ago
|
Status: NEW → ASSIGNED
Comment 4•25 years ago
|
||
I'm the man. cc'ing my manager.
Updated•25 years ago
|
Target Milestone: M9
Comment 5•25 years ago
|
||
moved this to M9, because the scope of the bug is very large and it will take a
lot of changes to fix, I think, but I'm going to make every effort to get this in
for M8.
Updated•25 years ago
|
Target Milestone: M9 → M8
Comment 6•25 years ago
|
||
Moved a less important bug so I could focus on this one for M8
Updated•25 years ago
|
Target Milestone: M8 → M9
Comment 7•25 years ago
|
||
rickg is ready to throw the switch on this as soon as (with jband's approval) I
give him the right allocator; however, he agrees---or even insists :-), there are
too many ramifications to put this into M8
Changing all Networking Library/Browser bugs to Networking-Core component for
Browser.
Occasionally, Bugzilla will burp and cause Verified bugs to reopen when I do
this in a bulk change. If this happens, I will fix. ;-)
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Comment 9•25 years ago
|
||
All right. I've checked in fixes to nsStr.h, nsCRT.h, and nsCRT.cpp to all use |
nsAllocator| via the |nsCppSharedAllocator| interface. |ToNewString| and |
nsCRT::Free()| at least agree now. Let's make sure other globally exchanged
objects cooperate as we encounter problems.
We'll want verification from bruce that this fix helps his Purify situation.
Assignee | ||
Comment 10•25 years ago
|
||
Re-opening this to track the results of this change and the fact that there are
a ton of delete[]'s scattered over the tree from nsString::ToNewCString() stuff.
See log at http://www.cybersight.com/~bruce/apprunner-necko19990726.log for the
fun.
Status: RESOLVED → REOPENED
Comment 11•25 years ago
|
||
Clearing resolution due to reopen.
Updated•25 years ago
|
Status: REOPENED → ASSIGNED
Comment 12•25 years ago
|
||
OK, we're going to try changing over to a global |operator new| and |delete| et
al so we can catch everything. Details to come.
Updated•25 years ago
|
Priority: P3 → P4
Comment 13•25 years ago
|
||
I'm marking 10502 as a duplicate of this bug. It basically says we need to go
through and find all the places that need to use the global allocator
Comment 14•25 years ago
|
||
*** Bug 10502 has been marked as a duplicate of this bug. ***
Comment 15•25 years ago
|
||
as instructed: changing all bugs that do not block M9 to M10
Assignee | ||
Updated•25 years ago
|
Assignee: scc → bruce
Status: ASSIGNED → NEW
Component: Networking-Core → XP Miscellany
Priority: P4 → P2
Target Milestone: M10 → M11
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 16•25 years ago
|
||
I've been fixing this ... might as well own it.
Updated•25 years ago
|
Target Milestone: M11 → M12
Comment 17•25 years ago
|
||
bruce, letme know if you have something in hand and if its low risk
we could take in in m11. looks like that most like is not the case.
Updated•25 years ago
|
Target Milestone: M12 → M14
Assignee | ||
Updated•25 years ago
|
Target Milestone: M14 → M15
Assignee | ||
Comment 18•25 years ago
|
||
I think this is mostly resolved .. I haven't seen these problems in a while.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
Component: String → XPCOM
You need to log in
before you can comment on or make changes to this bug.
Description
•