Closed Bug 1275898 Opened 4 years ago Closed 4 years ago

about:cache channels must start pumping only after AsyncOpen being called on them

Categories

(Core :: Networking: Cache, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file)

The true reason is a bit different.  Original reason to have bug 1266196 was because someone started to create (not open!) about:cache channels twice and they suffered from not being able to concur.

The reason we still see this crash is that the first channel starts pumping the data before it's even open.  And as it's not put to any load group it's not canceled during shutdown at all.

We need to start pumping only when the channel is actually asyncopened and also, to be sure, add a hard break to the background pump loop, just to be sure.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Summary: Be even more aggressive when canceling about:cache after shutdown → about:cache channels must start pumping only after AsyncOpen being called on them
- nsAboutCache::Channel::VisitNextStorage() is a method that starts the walking process
- it's no longer called from Init (time of creation of the channel) but correctly from reimplmented AsyncOpen*/Open*
- also rather added !shutdown condition to the background entry loop

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f114037255d
Attachment #8758237 - Flags: review?(michal.novotny)
Whiteboard: [necko-active]
Attachment #8758237 - Flags: review?(michal.novotny) → review+
Keywords: checkin-needed
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/137c977846ad
Proper about:cache asyncOpen implementation + kill the disk entries loop hard on shutdown, r=michal
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/137c977846ad
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8758237 [details] [diff] [review]
v1 (start walk on asyncOpen, kill loop on shutdown hard)

Approval Request Comment
[Feature/regressing bug #]: HTTP cache v2 + bug 1262009
[User impact if declined]: shutdown hang/crash when closing Fx and about:cache is still loading
[Describe test coverage new/current, TreeHerder]: few weeks on m-c, fixes other (previously hidden) cause of the shutdown hang
[Risks and why]: very low, this is only a diagnostic code, the patch changes the point we start internal loop feeding about:cache (doing a lot of IO) to a later and more proper place
[String/UUID change made/needed]: none

I also want to note that these crashes are very frequent and the patch is a really low risk.
Attachment #8758237 - Flags: approval-mozilla-aurora?
Before landing on m-a please also follow dependencies, this must land over bug 1271701.
Depends on: 1271701
Comment on attachment 8758237 [details] [diff] [review]
v1 (start walk on asyncOpen, kill loop on shutdown hard)

Diagnostic patch for shutdown hang, ok to uplift to beta. The patch in bug 1271701 is already in 48. And this missed the merge cutoff, so is already landed in aurora.
Attachment #8758237 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.