Closed
Bug 1273875
Opened 8 years ago
Closed 8 years ago
During shutdown leak as much as we can from HTTP cache
Categories
(Core :: Networking: Cache, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: mayhemer, Assigned: mayhemer)
Details
(Whiteboard: [necko-active])
Attachments
(2 files)
1.50 KB,
patch
|
michal
:
review+
|
Details | Diff | Splinter Review |
1.50 KB,
patch
|
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
The plan is to leave the list of active entries and the pools unfreed similarly to https://bugzilla.mozilla.org/attachment.cgi?id=8751204&action=diff#a/netwerk/cache2/CacheFileUtils.cpp_sec2
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=947457c35e8a (there are more patches in the series, should be fine, tho)
Assignee | ||
Updated•8 years ago
|
Attachment #8753832 -
Flags: review?(michal.novotny)
Comment 2•8 years ago
|
||
Comment on attachment 8753832 [details] [diff] [review] v1 (leak hashtables on !NS_FREE_PERMANENT_DATA) With this change we won't release CacheEntry and the associated CacheFile. So if the entry is complete but the metadata has not been written via CacheFileIOManager::ScheduleMetadataWrite then we'll leave invalid file on the disk, right?
Flags: needinfo?(honzab.moz)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(honzab.moz)
Whiteboard: [necko-active]
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to Michal Novotny (:michal) from comment #2) > Comment on attachment 8753832 [details] [diff] [review] > v1 (leak hashtables on !NS_FREE_PERMANENT_DATA) > > With this change we won't release CacheEntry and the associated CacheFile. > So if the entry is complete but the metadata has not been written via > CacheFileIOManager::ScheduleMetadataWrite then we'll leave invalid file on > the disk, right? Ups, didn't want to drop the ni flag. Yes, and we already do that after bug 913822 and its followups. As I understand, this is a correct thing to do.
Updated•8 years ago
|
Attachment #8753832 -
Flags: review?(michal.novotny) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 5•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/61d37541894f
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8753832 [details] [diff] [review] v1 (leak hashtables on !NS_FREE_PERMANENT_DATA) Approval Request Comment [Feature/regressing bug #]: http cache v2 [User impact if declined]: shutdown hang/crash [Describe test coverage new/current, TreeHerder]: on m-c for a week, heavily used code [Risks and why]: extremely low, this just leaks memory (when we don't do leak checking) during shutdown [String/UUID change made/needed]: none
Attachment #8753832 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•8 years ago
|
Attachment #8753832 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 7•8 years ago
|
||
Approval Request Comment, see comment 6
Attachment #8759606 -
Flags: approval-mozilla-aurora?
Updated•8 years ago
|
status-firefox48:
--- → affected
Comment 8•8 years ago
|
||
mayhemer, will this still apply to beta 48? This missed uplift before the last merge.
Flags: needinfo?(honzab.moz)
Comment 9•8 years ago
|
||
Comment on attachment 8759606 [details] [diff] [review] [m-a version] v1 (leak hashtables on !NS_FREE_PERMANENT_DATA) Switching approval flag to 48 beta.
Attachment #8759606 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #8) > mayhemer, will this still apply to beta 48? This missed uplift before the > last merge. It's very likely it will still apply. I cannot verify by applying the patch, I don't have beta checked out, but looking at the changes and beta state tells me it's OK to go.
Flags: needinfo?(honzab.moz)
Comment 11•8 years ago
|
||
Comment on attachment 8759606 [details] [diff] [review] [m-a version] v1 (leak hashtables on !NS_FREE_PERMANENT_DATA) May help with shutdown hangs, let's uplift it.
Attachment #8759606 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/36122c034b7e
You need to log in
before you can comment on or make changes to this bug.
Description
•