Closed Bug 457215 Opened 13 years ago Closed 13 years ago
lcms leaks 12,288 bytes on exit
lcms leaks 12,288 bytes on exit. STEPS TO REPRODUCE 1. Build Firefox (x86_64 Linux) with: ac_add_options --enable-jemalloc ac_add_options --with-valgrind 2. run Firefox under valgrind (firefox -g -d valgrind) 3. File->Quit ACTUAL RESULTS ==27750== 12,288 bytes in 3 blocks are definitely lost in loss record 160 of 168 ==27750== at 0x4082B7: arena_malloc_large (jemalloc.c:3939) ==27750== by 0x408775: arena_malloc (jemalloc.c:3964) ==27750== by 0x408E5E: imalloc (jemalloc.c:3974) ==27750== by 0x409C91: malloc (jemalloc.c:5983) ==27750== by 0x1493A56E: _cmsMalloc (lcms.h:1438) ==27750== by 0x1493A8C0: cmsAllocGamma (cmsgamma.c:124) ==27750== by 0x1493AA5C: cmsDupGamma (cmsgamma.c:159) ==27750== by 0x1494350A: cmsReadICCGamma (cmsio1.c:1815) ==27750== by 0x14959E77: cmsPrecacheProfile (cmsprecache.c:111) ==27750== by 0x146A6658: gfxPlatform::GetCMSsRGBProfile() (gfxPlatform.cpp:641) ==27750== by 0x146A68D4: gfxPlatform::GetCMSOutputProfile() (gfxPlatform.cpp:620) ==27750== by 0x14B79CEE: info_callback(png_struct_def*, png_info_struct*) (nsPNGDecoder.cpp:566) ==27750== by 0x14B95C61: MOZ_PNG_push_have_info (pngpread.c:1728) ==27750== by 0x14B97180: MOZ_PNG_push_read_chunk (pngpread.c:410) ==27750== by 0x14B9755B: MOZ_PNG_proc_some_data (pngpread.c:55) ==27750== by 0x14B975BD: MOZ_PNG_process_data (pngpread.c:35) ==27750== by 0x14B78AF6: ReadDataOut(nsIInputStream*, void*, char const*, unsigned, unsigned, unsigned*) (nsPNGDecoder.cpp:338) ==27750== by 0x57946C2: nsPipeInputStream::ReadSegments(unsigned (*)(nsIInputStream*, void*, char const*, unsigned, unsigned, unsigned*), void*, unsigned, unsigned*) (nsPipe3.cpp:799) ==27750== by 0x14B7893F: nsPNGDecoder::WriteFrom(nsIInputStream*, unsigned, unsigned*) (nsPNGDecoder.cpp:368) ==27750== by 0x14B74F7C: imgRequest::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned, unsigned) (imgRequest.cpp:878) ==27750== by 0x14B6DD3C: ProxyListener::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned, unsigned) (imgLoader.cpp:1400) ==27750== by 0x12EFC691: nsJARChannel::OnDataAvailable(nsIRequest*, nsISupports*, nsIInputStream*, unsigned, unsigned) (nsJARChannel.cpp:872) ==27750== by 0x125723B6: nsInputStreamPump::OnStateTransfer() (nsInputStreamPump.cpp:508) ==27750== by 0x125724DF: nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (nsInputStreamPump.cpp:398) ==27750== by 0x5795D68: nsInputStreamReadyEvent::Run() (nsStreamUtils.cpp:111) ==27750== by 0x57AF95F: nsThread::ProcessNextEvent(int, int*) (nsThread.cpp:510) ==27750== by 0x5771B03: NS_ProcessNextEvent_P(nsIThread*, int) (nsThreadUtils.cpp:227) ==27750== by 0x152363A1: nsBaseAppShell::Run() (nsBaseAppShell.cpp:170) ==27750== by 0x1B150251: nsAppStartup::Run() (nsAppStartup.cpp:182) ==27750== by 0x50606CC: XRE_main (nsAppRunner.cpp:3265) ==27750== by 0x401BD6: main (nsBrowserApp.cpp:156) ==27750==
I have verified that ShutdownCMS() is called by adding a printf: printf("ShutdownCMS: gCMSOutputProfile=%p gCMSsRGBProfile=%p\n", gCMSOutputProfile, gCMSsRGBProfile); and got: ShutdownCMS: gCMSOutputProfile=0x1f831000 gCMSsRGBProfile=0x1f831000 so it appears the leak is within lcms.
Seems plausible enough. Looks like there are some issues with the freeing for the precache code. Assigning to myself.
Assignee: nobody → bholley
Added a patch. I fixed this issue some time ago, but I for I guess it was in a patch that never landed. Mats, can you confirm that this plugs the leak? I don't have a linux box so I can't valgrind.
FYI, I got this compile warning (unrelated to your patch): lcms.h: In function '_cmsMalloc': lcms.h:1436: warning: comparison of unsigned expression < 0 is always false
Comment on attachment 340770 [details] [diff] [review] fix Ok - flagging vlad for review Those warnings have always been in LCMS. The problem is that lcms routes mallocs through a macro that checks to see if the malloc-ed amount is < 0. When an unsigned is passed in, it complains. I'll fix it sometime, but it's harmless.
Attachment #340770 - Flags: review?(vladimir)
Attachment #340770 - Flags: review?(vladimir) → review+
Comment on attachment 340770 [details] [diff] [review] fix Seeking Approval-1.9.1. This patch fixes a memory leak in lcms, which is important for 3.1 because people generally complain if they find their browser doesn't free all its memory on shutdown, and could lead to negative press. This has very low regression risk, since it's 3 lines that free a pointer that was allocated but never freed.
Comment on attachment 340770 [details] [diff] [review] fix a191=beltzner
Attachment #340770 - Flags: approval1.9.1? → approval1.9.1+
Added a version of the patch with user and commit messages set for metered checkin.
Screwed up the username string. Updating.
Attachment #350413 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Backed out to investigate bug 467102: http://hg.mozilla.org/mozilla-central/rev/3957258880e7
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Component: GFX → GFX: Color Management
Product: Core Graveyard → Core
QA Contact: general → color-management
um, why did you flag this checkin-needed Dao? It landed finally on both trees (before the branch) almost half a year ago.
also FWIW LCMS is obsoleted by bug 481926.
Trying to make the "has 1.9.1 approval but didn't land" list useful...
You need to log in before you can comment on or make changes to this bug.