New issue
Advanced search Search tips

Issue 834033 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-05-09
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

potential memory leak in ExtensionActionAPI

Project Member Reported by erikc...@chromium.org, Apr 17 2018

Issue description

Out of Process Heap Profiling is a project that, for a very small set users, records heap allocations and sends back anonymized heap dumps. 

In the last week, we received 209 reports of a potential leak. 

Sample heap dumps ids:
  313edc006ec3d661
  773e952c73fd4a31
  1a07fe075e2481f4
  66652e982e4e021f
  efeac0505ecfd034

The largest report had 315723 objects that were allocated but not destroyed, taking up 806MB of memory.

        806831408 bytes / 315723 objects
        profiling::(anonymous namespace)::HookAlloc(base::allocator::AllocatorDispatch const*, unsigned long, void*)
        base::allocator::MallocZoneFunctionsToReplaceDefault()::$_1::__invoke(_malloc_zone_t*, unsigned long)
        <???>
        <???>
        base::allocator::UncheckedMallocMac(unsigned long, void**)
        sk_malloc_flags(unsigned long, unsigned int)
        SkMallocPixelRef::MakeAllocate(SkImageInfo const&, unsigned long)
        SkBitmap::tryAllocPixels(SkImageInfo const&, unsigned long)
        IPC::ParamTraits<SkBitmap>::Read(base::Pickle const*, base::PickleIterator*, SkBitmap*)
        ExtensionAction::ParseIconFromCanvasDictionary(base::DictionaryValue const&, gfx::ImageSkia*)
        extensions::ExtensionActionSetIconFunction::RunExtensionAction()
        extensions::ExtensionActionFunction::Run()
        ExtensionFunction::RunWithValidation()
        extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int, base::RepeatingCallback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, extensions::functions::HistogramValue)> const&)
        extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int)
        bool IPC::MessageT<ExtensionHostMsg_Request_Meta, std::__1::tuple<ExtensionHostMsg_Request_Params>, void>::Dispatch<extensions::ExtensionWebContentsObserver, extensions::ExtensionWebContentsObserver, content::RenderFrameHost, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)>(IPC::Message const*, extensions::ExtensionWebContentsObserver*, extensions::ExtensionWebContentsObserver*, content::RenderFrameHost*, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&))
        extensions::ExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*)
        extensions::ChromeExtensionWebContentsObserver::OnMessageReceived(IPC::Message const&, content::RenderFrameHost*)
        content::WebContentsImpl::OnMessageReceived(content::RenderFrameHostImpl*, IPC::Message const&)
        content::RenderFrameHostImpl::OnMessageReceived(IPC::Message const&)
        IPC::ChannelProxy::Context::OnDispatchMessage(IPC::Message const&)
        base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
        base::MessageLoop::RunTask(base::PendingTask*)
        base::MessageLoop::DoWork()
        base::MessagePumpCFRunLoopBase::RunWork()
        base::mac::CallWithEHFrame(void () block_pointer)
        base::MessagePumpCFRunLoopBase::RunWorkSource(void*)
        <???>
        <???>
        <???>
        <???>
        <???>
        <???>
        <???>
        <???>
        <???>
        __71-[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]_block_invoke
        base::mac::CallWithEHFrame(void () block_pointer)
        -[BrowserCrApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
        <???>
        base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*)
        base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*)
        <name omitted>
        ChromeBrowserMainParts::MainMessageLoopRun(int*)
        content::BrowserMainLoop::RunMainMessageLoopParts()
        content::BrowserMainRunnerImpl::Run()
        content::BrowserMain(content::MainFunctionParams const&)
        content::ContentMainRunnerImpl::Run()
        service_manager::Main(service_manager::MainParams const&)
        content::ContentMain(content::ContentMainParams const&)
        ChromeMain
        main
        <???>

 Allocators:
     blink_gc: size=0 virtual_size=0
     blink_gc/allocated_objects: shim_allocated_objects_count=0 shim_allocated_objects_size=0
     malloc: size=1855707827 virtual_size=1855707827
     malloc/allocated_objects: shim_allocated_objects_count=11360475 shim_allocated_objects_size=1855707827
     partition_alloc: size=0 virtual_size=0
     partition_alloc/allocated_objects: shim_allocated_objects_count=0 shim_allocated_objects_size=0

Documentation: https://docs.google.com/document/d/1-Mvw-QkE2NsBnyoJzXoj_y-CgIRfslxAffg4j7fgdMk/edit?ts=5ad0d443#
 
Owner: erikc...@chromium.org
Status: Assigned (was: Untriaged)
over to myself to write analysis on how to triage these types of bugs.
Cc: erikc...@chromium.org
Components: Platform>Extensions
Owner: rdevlin....@chromium.org
Summary: potential memory leak in ExtensionActionAPI (was: potential memory leak in base::allocator::UncheckedMallocMac awaiting triage)
Icons are stored in a map on extension_action_. There can be 1 icon set for each tab. But they are only cleared on tab navigation, not on tab close. As a result, a user that opens/closes many tabs [especially if an extension is doing it!] can result in a large # of icons that are never cleared.

For a detailed analysis, see: https://chromium.googlesource.com/chromium/src/+/HEAD/docs/memory/investigating_heap_dump_example.md
Labels: Performance-Memory
Project Member

Comment 4 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cde631b4d13dbf472b2b9a3e2a2f0244b8a19b45

commit cde631b4d13dbf472b2b9a3e2a2f0244b8a19b45
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue May 08 01:53:05 2018

[Extensions] Clear ExtensionAction values on tab removal

ExtensionAction values can be set for specific tab ids. When these tabs
are removed, the values should be cleared. Add a regression test for
the same.

Bug:  834033 
Change-Id: I5eb6c398657ef307a5ceeea8116d4cb27d3f19ba
Reviewed-on: https://chromium-review.googlesource.com/1048632
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556641}
[modify] https://crrev.com/cde631b4d13dbf472b2b9a3e2a2f0244b8a19b45/chrome/browser/extensions/api/extension_action/extension_action_apitest.cc
[modify] https://crrev.com/cde631b4d13dbf472b2b9a3e2a2f0244b8a19b45/chrome/browser/extensions/tab_helper.cc
[modify] https://crrev.com/cde631b4d13dbf472b2b9a3e2a2f0244b8a19b45/chrome/browser/extensions/tab_helper.h

NextAction: 2018-05-09
Thanks for the report, Erik!  I think this should be fixed with #4.  With Out of Process Heap Profiling, is there a way to verify this in the wild?  (similar to how we would check that crash rates go to 0 to verify a crash is fixed.)
Cc: etienneb@chromium.org
Great! Thanks for landing a fix. :)

etienneb@ is working on automatically updating bugs with frequency of occurence/magnitude in wild, segmented by chrome version #, etc.
Status: Fixed (was: Assigned)
That sounds pretty awesome!

For now, I'll go ahead and close this out, but if/when that happens and confirms (or disproves), I'd be very interested!
The NextAction date has arrived: 2018-05-09

Sign in to add a comment