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

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Honza Bambas (:mayhemer)]

https://hg.mozilla.org/mozilla-central/annotate/8d0aadfe7da7/netwerk/cache2/CacheStorageService.cpp#l289 should also stop when after shutdown.

Comment 1

[Honza Bambas (:mayhemer)]

Err: https://hg.mozilla.org/mozilla-central/annotate/8d0aadfe7da7/netwerk/cache2/CacheStorageService.cpp#l458

here is the place...

Comment 2

[Honza Bambas (:mayhemer)]

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.

Comment 3

[Honza Bambas (:mayhemer)]

Attachment: v1 (start walk on asyncOpen, kill loop on shutdown hard)

- 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

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/137c977846ad
Proper about:cache asyncOpen implementation + kill the disk entries loop hard on shutdown, r=michal

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/137c977846ad

Comment 6

[Honza Bambas (:mayhemer)]

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.

Comment 7

[Honza Bambas (:mayhemer)]

Before landing on m-a please also follow dependencies, this must land over bug 1271701.

Comment 8

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2a762ba88494