Closed
Bug 457215
Opened 15 years ago
Closed 14 years ago
lcms leaks 12,288 bytes on exit
Categories
(Core :: Graphics: Color Management, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b3
People
(Reporter: MatsPalmgren_bugz, Assigned: bholley)
References
Details
(Keywords: fixed1.9.1, memory-leak, valgrind)
Attachments
(2 files, 1 obsolete file)
769 bytes,
patch
|
vlad
:
review+
beltzner
:
approval1.9.1+
|
Details | Diff | Splinter Review |
872 bytes,
patch
|
Details | Diff | Splinter Review |
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==
Reporter | ||
Comment 1•15 years ago
|
||
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.
Assignee | ||
Comment 2•15 years ago
|
||
Seems plausible enough. Looks like there are some issues with the freeing for the precache code. Assigning to myself.
Assignee: nobody → bholley
Assignee | ||
Comment 3•15 years ago
|
||
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.
Reporter | ||
Comment 4•15 years ago
|
||
Comment on attachment 340770 [details] [diff] [review] fix This fixed it for me.
Reporter | ||
Comment 5•15 years ago
|
||
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
Assignee | ||
Comment 6•15 years ago
|
||
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+
Assignee | ||
Updated•14 years ago
|
Attachment #340770 -
Flags: approval1.9.1?
Assignee | ||
Comment 7•14 years ago
|
||
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 8•14 years ago
|
||
Comment on attachment 340770 [details] [diff] [review] fix a191=beltzner
Attachment #340770 -
Flags: approval1.9.1? → approval1.9.1+
Assignee | ||
Comment 9•14 years ago
|
||
Added a version of the patch with user and commit messages set for metered checkin.
Assignee | ||
Comment 10•14 years ago
|
||
Screwed up the username string. Updating.
Attachment #350413 -
Attachment is obsolete: true
Comment 11•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/527249758771
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Comment 12•14 years ago
|
||
Backed out to investigate bug 467102: http://hg.mozilla.org/mozilla-central/rev/3957258880e7
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 13•14 years ago
|
||
relanded http://hg.mozilla.org/mozilla-central/rev/fc1c0104ec4d
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Product: Core → Core Graveyard
Assignee | ||
Updated•14 years ago
|
Component: GFX → GFX: Color Management
Product: Core Graveyard → Core
QA Contact: general → color-management
Updated•14 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 14•14 years ago
|
||
um, why did you flag this checkin-needed Dao? It landed finally on both trees (before the branch) almost half a year ago.
Assignee | ||
Comment 15•14 years ago
|
||
also FWIW LCMS is obsoleted by bug 481926.
Comment 16•14 years ago
|
||
Trying to make the "has 1.9.1 approval but didn't land" list useful...
Keywords: checkin-needed → fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•