1513676 - Twice-reported memory allocations in CacheStorageService code

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P2)

Description

[Kartikaya Gupta (email:[e-mail])]

+++ This bug was initially created as a clone of Bug #1513672 +++

I ran DMD today on a WR-enabled mac build to chase down bug 1491512 a bit more, and the DMD report told me about some twice-reported blocks. There is the entry below in CacheStorageService code. Full log is attached to bug 1513672.

Twice-reported {
  1 block in heap block record 5 of 11
  304 bytes (296 requested / 8 slop)
  0.00% of the heap (0.00% cumulative)
  3.98% of twice-reported (85.74% cumulative)
  Allocated at {
    #01: replace_calloc(unsigned long, unsigned long) (DMD.cpp:1136, in libmozglue.dylib)
    #02: _PR_CreateThread (ptthread.c:359, in libnss3.dylib)
    #03: PR_CreateThread (ptthread.c:518, in libnss3.dylib)
    #04: mozilla::net::CacheFileIOManager::InitInternal() (CacheIOThread.cpp:261, in XUL)
    #05: mozilla::net::CacheFileIOManager::Init() (CacheFileIOManager.cpp:1136, in XUL)
    #06: mozilla::net::CacheObserver::Observe(nsISupports*, char const*, char16_t const*) (CacheObserver.cpp:428, in XUL)
    #07: nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) (nsObserverList.cpp:65, in XUL)
    #08: nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) (TimeStamp.h:452, in XUL)
  }
  Reported at {
    #01: mozilla::dmd::ReportHelper(void const*, bool) (DMD.cpp:425, in libmozglue.dylib)
    #02: ThreadsReporter::MallocSizeOf(void const*) (nsMemoryReporterManager.cpp:1329, in XUL)
    #03: nsThread::ShallowSizeOfIncludingThis(unsigned long (*)(void const*)) const (nsThread.cpp:1000, in XUL)
    #04: ThreadsReporter::CollectReports(nsIHandleReportCallback*, nsISupports*, bool) (nsMemoryReporterManager.cpp:1361, in XUL)
    #05: mozilla::detail::RunnableFunction<nsMemoryReporterManager::DispatchReporter(nsIMemoryReporter*, bool, nsIHandleReportCallback*, nsISupports*, bool)::$_0>::Run() (nsMemoryReporterManager.cpp:1850, in XUL)
    #06: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:1144, in XUL)
    #07: NS_ProcessPendingEvents(nsIThread*, unsigned int) (nsThreadUtils.cpp:416, in XUL)
    #08: nsBaseAppShell::NativeEventCallback() (nsBaseAppShell.cpp:88, in XUL)
  }
  Reported again at {
    #01: mozilla::dmd::ReportHelper(void const*, bool) (DMD.cpp:425, in libmozglue.dylib)
    #02: mozilla::net::CacheStorageService::MallocSizeOf(void const*) (CacheStorageService.h:132, in XUL)
    #03: mozilla::net::CacheIOThread::SizeOfExcludingThis(unsigned long (*)(void const*)) const (CacheIOThread.cpp:599, in XUL)
    #04: mozilla::net::CacheFileIOManager::SizeOfExcludingThisInternal(unsigned long (*)(void const*)) const (CacheIOThread.cpp:612, in XUL)
    #05: mozilla::net::CacheFileIOManager::SizeOfIncludingThis(unsigned long (*)(void const*)) (CacheFileIOManager.cpp:4270, in XUL)
    #06: mozilla::net::CacheStorageService::CollectReports(nsIHandleReportCallback*, nsISupports*, bool) (nsTStringRepr.h:293, in XUL)
    #07: non-virtual thunk to mozilla::net::CacheStorageService::CollectReports(nsIHandleReportCallback*, nsISupports*, bool) (CacheStorageService.cpp:0, in XUL)
    #08: mozilla::detail::RunnableFunction<nsMemoryReporterManager::DispatchReporter(nsIMemoryReporter*, bool, nsIHandleReportCallback*, nsISupports*, bool)::$_0>::Run() (nsMemoryReporterManager.cpp:1850, in XUL)
  }
}

Comment 1

[Dragana Damjanovic [:dragana]]

Michal, can you take a look?

Comment 2

[Michal Novotny [:michal]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #0)
>   Reported at {
>     #01: mozilla::dmd::ReportHelper(void const*, bool) (DMD.cpp:425, in
> libmozglue.dylib)
>     #02: ThreadsReporter::MallocSizeOf(void const*)
> (nsMemoryReporterManager.cpp:1329, in XUL)
>     #03: nsThread::ShallowSizeOfIncludingThis(unsigned long (*)(void
> const*)) const (nsThread.cpp:1000, in XUL)

I'm not sure I read the log correctly. Could the twice reported item be CacheIOThread::mThread? Once from CacheIOThread::SizeOfExcludingThis and second time from https://searchfox.org/mozilla-central/rev/0ee0b63732d35d16ba22d5a1120622e2e8d58c29/xpcom/threads/nsThread.cpp#1000? IIUC, this could happen only when the thread is shutting down. Honza, what do you think?

Comment 3

[Honza Bambas (:mayhemer)]

Look like that's it.  I then means to not report nsThread instances "manually"?  Honestly, it makes sense.

So, this is just about removing this line:
https://searchfox.org/mozilla-central/rev/f8de61826903996f6bdf41b11a2844dd59ac144f/netwerk/cache2/CacheIOThread.cpp#599

Comment 4

[Michal Novotny [:michal]]

Attachment: fix

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c85b59bb06cb
Twice-reported memory allocations in CacheStorageService code, r=honzab

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c85b59bb06cb