Closed Bug 6926 Opened 25 years ago Closed 25 years ago

MLK: leak in nsProfile::GetCurrentProfileDir()

Categories

(Core Graveyard :: Profile: BackEnd, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bruce, Assigned: racham)

Details

Attachments

(1 file)

This leads to the largest leak in M6 (over time). nsGlobalHistory::GetHistoryDir() calls nsProfile::GetCurrentProfileDir(). GetCurrentProfileDir() calls GetCurrentProfile() with the address of a char* profileName. nsProfile::GetCurrentProfile() does: *profileName = (char*) PR_Malloc(sizeof(char) * _MAX_LENGTH); A couple of lines later, it does (when an operation has failed): profileName = '\0'; This would appear to be the source of the leak, as back up one stack frame in nsProfile::GetCurrentProfileDir(), it checks to see if profileName (a local var in that scope) is null or not, and if not, it PR_DELETE()s it. this assignment of profileName to '\0' while it is a char** would appear incorrect. If it is correct, then the memory needs to be freed up somewhere. For a related bug (and why this code is getting called so often), check out bug #6922.
selmer, racham, gayatrib; is there anyone to look at this over the weekend?
Okay. Nothing so complicated as all that. Found the problem and going to attach a patch in about 5 minutes. Don't know the profileName = '\0' thing is a problem or not, but m_reg->GetString() allocates memory, so you don't need to allocate memory before passing the char** into it.
Attached patch patch to fix huge memory leak (deleted) — Splinter Review
Well, looking around a bit more, there's another instance of this same thing happening elsewhere. Lines 933 and 934 of version 1.11 of nsProfile.cpp down in nsProfile::CopyRegKey() have the same errors. These mallocs should not be needed. A quick glance at all of the other uses of nsRegistry::GetString() don't seem to show any other violations.
Target Milestone: M6
bruce, do you want to check the patch in for M6?
sure. will do so today.
Fix checked in for the main problem. Don't want to mark this fixed until the whole file has been reviewed and fixed.
I nailed down the critical portion. Should we move the rest of the bug (the review of the rest of the file) to M7?
Target Milestone: M6 → M7
ok. -> m7. how big is the current leak after your first fix?
It's really a pain to have a single bug track multiple problems/actions. Bhuvan, please open a new bug for the review process and then mark this one fixed. Steve
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Bug #6984 to continue the review of the other segments of profile code.
Status: RESOLVED → VERIFIED
Component: Profile Manager → Profile Manager BackEnd
Moving all Profile Manager bugs to new Profile Manager Backend component. Profile Manager component to be deleted.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: