memory leak in nsDirPrefs.cpp

RESOLVED FIXED in Thunderbird 3.0b1

Status

RESOLVED FIXED
10 years ago
10 years ago

People

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

Tracking

Trunk
Thunderbird 3.0b1
x86
Solaris

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

10 years ago
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
                 ...
(Assignee)

Comment 1

10 years ago
Created attachment 339221 [details] [diff] [review]
patch
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?
(Assignee)

Comment 3

10 years ago
Created attachment 339224 [details] [diff] [review]
new patch
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+
(Assignee)

Comment 5

10 years ago
Created attachment 339577 [details] [diff] [review]
new patch based on Mark's comments
Attachment #339577 - Flags: superreview?(bienvenu)

Updated

10 years ago
Attachment #339577 - Flags: superreview?(bienvenu) → superreview+

Comment 6

10 years ago
Comment on attachment 339577 [details] [diff] [review]
new patch based on Mark's comments

thx, Boying
(Assignee)

Updated

10 years ago
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
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [checkin post beta 1 freeze]
Target Milestone: --- → Thunderbird 3.0b1

Comment 8

10 years ago
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.