As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 764773 - Gecko in B2G leaks around circa 500kB to 1MB/day with UI idling
: Gecko in B2G leaks around circa 500kB to 1MB/day with UI idling
Status: RESOLVED FIXED
[MemShrink]
: mlk
Product: Core
Classification: Components
Component: Hardware Abstraction Layer (HAL) (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla16
Assigned To: Cervantes Yu [:cyu] [:cervantes]
:
:
Mentors:
Depends on:
Blocks: 742226
  Show dependency treegraph
 
Reported: 2012-06-14 04:53 PDT by Julian Seward [:jseward]
Modified: 2012-07-24 13:34 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+


Attachments
Fix memory leak in UeventPoller.cpp (1.30 KB, patch)
2012-06-15 03:15 PDT, Cervantes Yu [:cyu] [:cervantes]
cjones.bugs: review+
Details | Diff | Splinter Review
Test code to verify the memleak is fixed. (2.02 KB, patch)
2012-06-15 03:16 PDT, Cervantes Yu [:cyu] [:cervantes]
no flags Details | Diff | Splinter Review

Description User image Julian Seward [:jseward] 2012-06-14 04:53:13 PDT
Following some Valgrinding of B2G on GalaxyS2, I am seeing the following
leak that occurs when the UI is idling (at the home screen), and whose size
appears to depend on how long it has been idling for.  My guesstimate of
the leak rate is 500kB/day to 1000kB/day.

The strdup in question isn't in our code, so I am wondering if there is
some way to release the memory allocated by NetlinkEvent::decode(char*, int, int),
that we need to do.

The numbers in this leak pertain to around 90 minutes of idling.  Assuming
that the leak size really is proportional to the length of idling, that is
a rate of pretty much 1MB/day.

62,586 bytes in 2,140 blocks are definitely lost in loss record 10,722 of 10,790
   at 0x48067E0: malloc (vg_replace_malloc.c:267)
   by 0x483335B: strdup (strdup.c:45)
   by 0x675B9ED: NetlinkEvent::parseAsciiNetlinkMessage(char*, int) (NetlinkEvent.cpp:202)
   by 0x675BBCB: NetlinkEvent::decode(char*, int, int) (NetlinkEvent.cpp:214)
   by 0x59C8C4F: mozilla::hal_impl::NetlinkPoller::OnFileCanReadWithoutBlocking(int) (UeventPoller.cpp:158)
   by 0x5A51305: base::MessagePumpLibevent::OnLibeventNotification(int, short, void*) (message_pump_libevent.cc:213)
   by 0x5A2984B: event_base_loop (event.c:385)
   by 0x5A513D9: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) (message_pump_libevent.cc:331)
   by 0x5A368BB: MessageLoop::RunInternal() (message_loop.cc:208)
   by 0x5A368C5: MessageLoop::RunHandler() (message_loop.cc:201)
   by 0x5A36999: MessageLoop::Run() (message_loop.cc:175)
   by 0x5A42BCF: base::Thread::ThreadMain() (thread.cc:156)
Comment 1 User image Julian Seward [:jseward] 2012-06-14 04:54:18 PDT
I should add, this is by far the largest "definitely lost" style leak
I see when running B2G on V.
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2012-06-14 07:35:13 PDT
The way NetlinkEvent works is that it sets some members (vis strdup) when decode() is called.  Those members are freed in its destructor.

The problem is that UeventPoller.cpp has a single NetlinkEvent object that it calls decode() on over and over and over.  So we get a leak, because only the strings from the last decode() call are freed when the poller goes away.

Cervantes, why do we need mNetlinkEvent at all?  Can we not just allocate a brand-new NetinkEvent on the stack every time before calling decode() and then Broadcast()?
Comment 3 User image Cervantes Yu [:cyu] [:cervantes] 2012-06-14 08:16:25 PDT
Boris, I just wanted to reuse the instance, but this turns out to be a bad idea since it strdup() instead of using the passed buffer. Allocating on the stack fixes the problem. Thanks for pointing this out.
Comment 4 User image Cervantes Yu [:cyu] [:cervantes] 2012-06-15 03:15:02 PDT
Created attachment 633452 [details] [diff] [review]
Fix memory leak in UeventPoller.cpp

Fix by not calling NetlinkEvent::decode() of the same instance repreatedly, which strdup() and only free() in dtor.
Comment 5 User image Cervantes Yu [:cyu] [:cervantes] 2012-06-15 03:16:16 PDT
Created attachment 633453 [details] [diff] [review]
Test code to verify the memleak is fixed.
Comment 6 User image Cervantes Yu [:cyu] [:cervantes] 2012-06-15 03:17:07 PDT
Test on try: https://tbpl.mozilla.org/?tree=Try&rev=dc46ebb36392
Comment 8 User image Ed Morley [:emorley] 2012-06-19 01:21:30 PDT
https://hg.mozilla.org/mozilla-central/rev/a9542323655e

Note You need to log in before you can comment on or make changes to this bug.