309729 - When the memory cache is disabled (through about:config) and then re-enabled in the same session, the capacity remains at zero.

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[Alfred Kayser]

Found during patching bug 308972:
When the memory cache is disabled (through about:cache) and then re-enabled in
the same session, the capacity remains at zero.

Comment 1

[Darin Fisher]

Did you mean "through about:config" instead?

Comment 2

[Alfred Kayser]

Yep, and I am investigating an issue with mTotalSize getting negative...

Comment 3

[Alfred Kayser]

Attachment: Patch to correctly disable/enable the memory cache device

Another patch to memory cache: fixing the enabling/disabling of memory cache,
by correctly 'SetCapacity' of the memory device, as well as more lazy memory
cache device creation (so that it only happens from one place, when the cache
service wants to store something in the memory cache).

Also moved 'CacheMemoryAvailable' to 'MemoryCacheCapacity' of the PrefObserver,
to be consistent with DiskCache, to move all 'preference' logic to the
'PrefObserver', and to reduce one member from nsServicCache.

Comment 4

[Darin Fisher]

Comment on attachment 197397 [details] [diff] [review]
Patch to correctly disable/enable the memory cache device

r=darin

Comment 5

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.  Thanks for the patch, Alfred!

Comment 6

[[e-mail]]

This hangs switching profiles.

Comment 7

[[e-mail]]

Comment on attachment 197397 [details] [diff] [review]
Patch to correctly disable/enable the memory cache device

>@@ -1164,39 +1218,37 @@ void
> nsCacheService::OnProfileChanged()
> {
>     if (!gService)  return;
>  
>     nsresult   rv = NS_OK;
>     nsAutoLock lock(gService->mCacheServiceLock);
<snip>
>+    gService->SetMemoryCache();
The problem is that SetMemoryCache also tries to lock the lock...

Comment 8

[Alfred Kayser]

Attachment: Additional patch to not lock, and to correctly reset 'MemoryCacheCapacity' to -1 if not set in profile

Additional patch to fix the lock problem when switching profiles.

* Move back the code within 'SetMemoryCache' to OnProfileChange to prevent
double locking
* Also reset mMemoryCacheCapacity to -1, if switching from profile with
capacity set to one without a set capacity.

Comment 9

[[e-mail]]

Comment on attachment 199145 [details] [diff] [review]
Additional patch to not lock, and to correctly reset 'MemoryCacheCapacity' to -1 if not set in profile

>-    nsresult   rv = NS_OK;
I don't think moving this declaration inside the #ifdef is a good idea. I don't
clam to understand the code, but it does at least prevent the hang, so marking
r+ for that.

Comment 10

[Alfred Kayser]

Comment on attachment 199145 [details] [diff] [review]
Additional patch to not lock, and to correctly reset 'MemoryCacheCapacity' to -1 if not set in profile

The 'rv' was moved, because it is only used within the ifdef itself

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 199145 [details] [diff] [review]
Additional patch to not lock, and to correctly reset 'MemoryCacheCapacity' to -1 if not set in profile

This probably needs a look from darin...

Comment 12

[Darin Fisher]

Comment on attachment 199145 [details] [diff] [review]
Additional patch to not lock, and to correctly reset 'MemoryCacheCapacity' to -1 if not set in profile

sr=darin

Comment 13

[Boris Zbarsky [:bzbarsky]]

Checked in that second patch on trunk.

Comment 14

[Tobias Fischer]

Thanks for fixing this. With Creature-Build 2005101120 my Problems with
different Profiles after Checkin from first Patch are solved, so I would like to
say its verified=fixed now. 

Comment 15

[benc]

V/fixed:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20051028 SeaMonkey/1.1a

set browser.cache.memory.enable to false.
about:cache - number of entries is zero.
set browser.cache.memory.enable to true.
about:cache - number immediately is non-zero.

changed profile as well.