Potential memory leak in chrome_browser_net::Predictor |
|||||||||
Issue descriptionThese results are produced by running an chrome extension that randomly browse the web for multiple days. Chrome is running with native-heap-profiling activated and memory allocations are tracked. The attached stackframes shows where the leaking memory got allocated. The bug was on windows 10 canary M61.
,
Aug 8 2017
see: https://cs.chromium.org/chromium/src/chrome/browser/net/predictor.cc?l=1003 UrlInfo* info = &results_[url]; info->SetUrl(url); // Initialize or DCHECK. // TODO(jar): I need to discard names that have long since expired. // Currently we only add to the domain map :-/ Data can be cleared with |Predictor::DiscardAllResults|. This is not called on a regular basis, thus |results_| keep growing over time.
,
Aug 8 2017
As shown in the bug1.png of #c1, there is about 25k objects leaked which is consuming 11.2M of memory.
,
Aug 8 2017
,
Aug 8 2017
,
Aug 8 2017
,
Aug 8 2017
,
Aug 8 2017
I briefly looked at the code and I think we can simply replace DnsProcessingState::ASSIGNED with DnsProcessingState::ASSIGNED_BUT_MARKED to ensure deletion after resolution. After resolution is complete I think the only consumer of the information is about:dns. lizeb@ could you take this on your plate?
,
Aug 9 2017
lizeb@: i am assigning this to you since you are actively working on Predictor. Please re-assign as needed.
,
Aug 10 2017
Assigning it to Alex who has looked at this most recently.
,
Aug 21 2017
,
Aug 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f644065628d4fc088f5deac6ab9464d350f7b361 commit f644065628d4fc088f5deac6ab9464d350f7b361 Author: Alexandr Ilin <alexilin@chromium.org> Date: Tue Aug 22 15:46:32 2017 net: Delete UrlInfo after DNS resolution The Predictor keeps in memory a UrlInfo object for all hosts that it tries to preresolve and never discards them (except if the user clears the browsing data). This causes memory leaks. After resolution is complete the map of UrlInfos is used to deduplicate DNS requests to already seen hosts that we beleive are still in the browser DNS cache. The another consumer of the information is about:dns. The additional layer of caching for DNS requests is unnecessary so this CL simply sets the DnsProcessingState::ASSIGNED_BUT_MARKED state instead of DnsProcessingState::ASSIGNED to ensure deletion in Predictor::LookupFinished(). Bug: 752954 , 757458 Change-Id: I8a6734b071f19ab289a7be5dedc7794266854e6d Reviewed-on: https://chromium-review.googlesource.com/612380 Commit-Queue: Alexandr Ilin <alexilin@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Helen Li <xunjieli@chromium.org> Cr-Commit-Position: refs/heads/master@{#496315} [modify] https://crrev.com/f644065628d4fc088f5deac6ab9464d350f7b361/chrome/browser/net/predictor.cc [modify] https://crrev.com/f644065628d4fc088f5deac6ab9464d350f7b361/chrome/browser/net/predictor.h [modify] https://crrev.com/f644065628d4fc088f5deac6ab9464d350f7b361/chrome/browser/net/predictor_browsertest.cc [modify] https://crrev.com/f644065628d4fc088f5deac6ab9464d350f7b361/chrome/browser/net/url_info.cc [modify] https://crrev.com/f644065628d4fc088f5deac6ab9464d350f7b361/chrome/browser/net/url_info.h
,
Aug 22 2017
Now it should be fixed. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by etienneb@chromium.org
, Aug 7 201749.1 KB
49.1 KB View Download
43.5 KB
43.5 KB View Download