Closed Bug 455847 Opened 13 years ago Closed 13 years ago

memory leak in nsDirPrefs.cpp

Categories

(Thunderbird :: Address Book, defect)

x86
Solaris
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b1

People

(Reporter: eagle.lu, Assigned: eagle.lu)

Details

Attachments

(1 file, 2 obsolete files)

Using solaris mdb+libumem.so, I detected a memory leak in nsDirPrefs.cpp.

Steps:
1. LD_PRELOAD=libumem.so
2. UMEM_DEBUG=default
3. LD_LIBRARY_PATH=dir-of-thunderbird-bin
4. export LD_PRELOAD UMEM_DEBUG LD_LIBRARY_PATH
5. mdb ./thunderbird-bin 
6  In mdb: 
   ::sysbp _exit
   ::run
7. write an e-mail to yourself
8. quit from TB
9. mdb stopped at _exit
   run ::findleaks in mdb

mdb reports following leak:
  libumem.so.1`umem_cache_alloc_debug+0x14f
                 libumem.so.1`umem_cache_alloc+0x144
                 libumem.so.1`umem_alloc+0xc5
                 libumem.so.1`malloc+0x27
                 libc.so.1`strdup+0x28
                 libaddrbook.so`char*DIR_GetStringPref+0x2cb
                 libaddrbook.so`void DIR_GetPrefsForOneServer+0x1d1
                 libaddrbook.so`unsigned dir_GetPrefs+0x1e6
                 libaddrbook.so`unsigned DIR_GetServerPreferences+0x157
                 libaddrbook.so`unsigned DIR_GetDirServers+0x51
                 libaddrbook.so`nsVoidArray*DIR_GetDirectories+0x34
                 libaddrbook.so`unsigned nsAbBSDirectory::EnsureInitialized+0x109
                 libaddrbook.so`unsigned nsAbBSDirectory::GetChildNodes+0x2f
                 libaddrbook.so`unsigned nsAbManager::GetDirectories+0x373
                 ...
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → brian.lu
Attachment #339221 - Flags: review?(bugzilla)
Comment on attachment 339221 [details] [diff] [review]
patch

Since the value here is a nsString (I guess it was a char* previously), can't you just assign defaltValue direct to value?
Attached patch new patch (obsolete) — Splinter Review
Attachment #339221 - Attachment is obsolete: true
Attachment #339224 - Flags: review?(bugzilla)
Attachment #339221 - Flags: review?(bugzilla)
Comment on attachment 339224 [details] [diff] [review]
new patch

+        value = defaultValue ? defaultValue : nsnull;

Since assigning nsnull to value is valid, this can just be:

value = defaultValue;

r=me with that fixed.
Attachment #339224 - Flags: review?(bugzilla) → review+
Attachment #339577 - Flags: superreview?(bienvenu)
Attachment #339577 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 339577 [details] [diff] [review]
new patch based on Mark's comments

thx, Boying
Keywords: checkin-needed
Whiteboard: [checkin post beta 1 freeze]
Attachment #339224 - Attachment is obsolete: true
Attachment #339577 - Flags: review+
Patch checked in, changeset id 488:febdebc0d555
Status: NEW → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [checkin post beta 1 freeze]
Target Milestone: --- → Thunderbird 3.0b1
Comment on attachment 339577 [details] [diff] [review]
new patch based on Mark's comments

>-    {
>-    }
Personally I wouldn't have removed these. (I'm surprised bienvenu didn't comment on this, he normally has to ask me to add them in.)
You need to log in before you can comment on or make changes to this bug.