455847 - memory leak in nsDirPrefs.cpp

Status: RESOLVED FIXED

Description

[Boying Lu]

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
                 ...

Comment 1

[Boying Lu]

Created attachment 339221 [details] [diff] [review]
patch

Comment 2

[Mark Banner (:standard8)]

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?

Comment 3

[Boying Lu]

Created attachment 339224 [details] [diff] [review]
new patch

Comment 4

[Mark Banner (:standard8)]

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.

Comment 5

[Boying Lu]

Created attachment 339577 [details] [diff] [review]
new patch based on Mark's comments

Comment 6

[David :Bienvenu]

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

thx, Boying

Comment 7

[Mark Banner (:standard8)]

Patch checked in, changeset id 488:febdebc0d555

Comment 8

[[e-mail]]

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.)