922123 - Shutdown hang with 100% CPU on Nightly with new cache

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[:Irving Reid (No longer working on Firefox)]

Attachment: Activity Monitor sample of cache clearing hang at shutdown

I turned on the pref for the new network cache (browser.cache.use_new_backend = 1); during a regular restart for Nightly update, the browser hung after the windows all closed. One core was running at full usage. an Activity Monitor trace is attached; it shows the Cache Mngmnt thread looping below mozilla::net::CacheStorageService::PurgeOverMemoryLimit().

I attached a debugger and tried to follow the loop, but I wasn't picking up symbols correctly so I couldn't clearly see what was going on, and then xcode crashed and killed the browser so I couldn't debug any more.

Comment 1

[Honza Bambas (:mayhemer)]

Seems like often reproducible.  I will take a look ASAP.  Thanks.

(Bad you were not able to debug, I was trying to reproduce this my self, no luck).

Comment 2

[Honza Bambas (:mayhemer)]

This could be a bug in nsTArray.  CacheStorageService::PurgeExpired loops and expects entries on which PurgeAndDoom is called gets removed from the array.  But apparently this doesn't happen and the code loops indefinitely.

Workaround is to remove the entry by index directly in CacheStorageService::PurgeExpired().  However, the entry is doing RemoveElement(this) on the array, but there is the bug - this doesn't work all the time.

I'll report a bug and in the mean time I'll fix this by a direct delete.

Comment 4

[Honza Bambas (:mayhemer)]

Attachment: v1

- the problem is not (cannot be) in nsTArray
- my theory (since I cannot reproduce) is that the registration flags are messed up by thread concurrency
- since the registry flags are accessed only on a single thread, I wanted to avoid unnecessary locking
- hence the flags are moved to a separate state var (also this code is better understandable)

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 812699 [details] [diff] [review]
v1

https://tbpl.mozilla.org/?tree=Try&rev=5e32180abf84

Comment 6

[Honza Bambas (:mayhemer)]

Comment on attachment 812699 [details] [diff] [review]
v1

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1d8f1dba4d5

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d1d8f1dba4d5