Twice-reported memory allocations in CacheStorageService code

RESOLVED FIXED in Firefox 66

Status

()

defect
P2
normal
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: kats, Assigned: michal)

Tracking

65 Branch
mozilla66
Points:
---

Firefox Tracking Flags

(firefox66 fixed)

Details

(Whiteboard: [necko-triaged])

Attachments

(1 attachment)

+++ 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)
  }
}
See Also: → 1513678
Michal, can you take a look?
Flags: needinfo?(michal.novotny)
Priority: -- → P2
Whiteboard: [necko-triaged]
Assignee

Updated

5 months ago
Assignee: nobody → michal.novotny
Flags: needinfo?(michal.novotny)
Assignee

Comment 2

5 months ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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?
Flags: needinfo?(honzab.moz)
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
Flags: needinfo?(honzab.moz)
Assignee

Comment 4

4 months ago
Posted patch fixSplinter Review
Attachment #9035190 - Flags: review?(honzab.moz)
Attachment #9035190 - Flags: review?(honzab.moz) → review+
Assignee

Updated

4 months ago
Keywords: checkin-needed

Comment 5

4 months ago

Pushed by aiakab@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c85b59bb06cb
Twice-reported memory allocations in CacheStorageService code, r=honzab

Keywords: checkin-needed

Comment 6

4 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.