1379080 - Crash in MatchEntry

Status: RESOLVED DUPLICATE of bug 1382688

(Core :: Networking: Cache, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-2469564c-0119-470b-8006-f80690170706.
=============================================================
Crashing Thread (40), Name: Cache I/O
Frame 	Module 	Signature 	Source
0 	xul.dll 	MatchEntry 	netwerk/cache/nsDiskCacheBinding.cpp:38
1 	xul.dll 	PLDHashTable::SearchTable<0>(void const*, unsigned __int64) 	xpcom/ds/PLDHashTable.cpp:398
2 	xul.dll 	PLDHashTable::Search(void const*) 	xpcom/ds/PLDHashTable.cpp:530
3 	xul.dll 	nsDiskCacheBindery::FindActiveBinding(unsigned int) 	netwerk/cache/nsDiskCacheBinding.cpp:200
4 	xul.dll 	nsDiskCacheDevice::FindEntry(nsCString*, bool*) 	netwerk/cache/nsDiskCacheDevice.cpp:473
5 	xul.dll 	nsCacheService::SearchCacheDevices(nsCString*, int, bool*) 	netwerk/cache/nsCacheService.cpp:2223
6 	xul.dll 	nsCacheService::ActivateEntry(nsCacheRequest*, nsCacheEntry**, nsCacheEntry**) 	netwerk/cache/nsCacheService.cpp:2121
7 	xul.dll 	nsCacheService::ProcessRequest(nsCacheRequest*, bool, nsICacheEntryDescriptor**) 	netwerk/cache/nsCacheService.cpp:1948
8 	xul.dll 	nsCacheService::OpenCacheEntry(nsCacheSession*, nsACString const&, int, bool, nsICacheListener*, nsICacheEntryDescriptor**) 	netwerk/cache/nsCacheService.cpp:2079
9 	xul.dll 	nsCacheSession::AsyncOpenCacheEntry(nsACString const&, int, nsICacheListener*, bool) 	netwerk/cache/nsCacheSession.cpp:105
10 	xul.dll 	mozilla::net::_OldCacheLoad::Run() 	netwerk/cache2/OldWrappers.cpp:776
11 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1437
12 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:489
13 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:339
14 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:313
15 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:293
16 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:506
17 	nss3.dll 	PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
18 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
19 	ucrtbase.dll 	o__strtoui64 	
20 	kernel32.dll 	BaseThreadInitThunk 	
21 	ntdll.dll 	RtlUserThreadStart

crashes with this signature are regressing since 56.0a1 build 20170705095941.

Comment 1

[Michal Novotny [:michal]]

This is a crash in an old cache code which isn't use anymore unless user enables it in about:config. We're not going to do any change in this code.

Comment 2

[[:philipp]]

which preference controls this (in case an affected user ends up reading this bug)?

Comment 3

[Michal Novotny [:michal]]

browser.cache.use_new_backend

Comment 4

[Honza Bambas (:mayhemer)]

(In reply to Michal Novotny (:michal) from comment #3)
> browser.cache.use_new_backend

To be more precise, the old backend is used iff:

browser.cache.use_new_backend == false && browser.cache.use_new_backend_temp == 0

The crash is really the old disk cache (not appcache!), so I can confirm a WONTFIX here.

Comment 5

[Chris Peterson [:cpeterson]]

(In reply to Michal Novotny (:michal) from comment #1)
> This is a crash in an old cache code which isn't use anymore unless user
> enables it in about:config. We're not going to do any change in this code.

Are there plans to remove this old disk cache code or disable the pref check? This MatchEntry crash signature appears to be new in Nightly 56. From the 100+ crash reports, it looks like about a dozen different users (unique install times) were affected.

Comment 6

[Michal Purzynski [:michal`] (use NEEDINFO)]

You're looking for a different Michal ;)

Comment 7

[Chris Peterson [:cpeterson]]

(In reply to Michal Purzynski [:michal`] (use NEEDINFO) from comment #6)
> You're looking for a different Michal ;)

oops. Sorry!

(In reply to Michal Novotny (:michal) from comment #1)
> This is a crash in an old cache code which isn't use anymore unless user
> enables it in about:config. We're not going to do any change in this code.

Michal, are there plans to remove this old disk cache code or disable the pref check? This MatchEntry crash signature appears to be new in Nightly 56. From the 100+ crash reports, it looks like about a dozen different users (unique install times) were affected.

Comment 8

[Michal Novotny [:michal]]

(In reply to Chris Peterson [:cpeterson] from comment #7)
> Michal, are there plans to remove this old disk cache code or disable the
> pref check? This MatchEntry crash signature appears to be new in Nightly 56.
> From the 100+ crash reports, it looks like about a dozen different users
> (unique install times) were affected.

We plan to remove the old code once we remove the app cache (see bug 913828). There is IMO no reason to allow anybody to use the old cache because it's now obsolete and unmaintained. We should simply ignore the pref and return always true in CacheObserver::UseNewCache(). Honza, what's your opinion?

Comment 9

[Honza Bambas (:mayhemer)]

I first want to figure out why this has regressed recently.

AppCache is not about to be removed any time sooner, big players still use it.

I'm afraid it would be too much work to remove this particular code that is crashing here and preserve appcache to work.

Comment 10

[Michal Novotny [:michal]]

(In reply to Honza Bambas (:mayhemer) from comment #9)
> I'm afraid it would be too much work to remove this particular code that is
> crashing here and preserve appcache to work.

Agree, that's why I proposed to ignore browser.cache.use_new_backend pref and not to remove old disk and memory cache code while keeping app cache.

Comment 11

[Honza Bambas (:mayhemer)]

(In reply to Michal Novotny (:michal) from comment #10)
> (In reply to Honza Bambas (:mayhemer) from comment #9)
> > I'm afraid it would be too much work to remove this particular code that is
> > crashing here and preserve appcache to work.
> 
> Agree, that's why I proposed to ignore browser.cache.use_new_backend pref
> and not to remove old disk and memory cache code while keeping app cache.

Yes, but it's pretty weird we suddenly started to crash in a code that should not be used at all.  Something has changed and I would like to know what exactly.

Comment 12

[Michal Novotny [:michal]]

Not sure what you exactly mean. If you believe that the code is called even when the old cache is disabled by the pref, then this could be easily verified by ignoring the pref. If the crash persists, we have a problem. If it's gone, then this is WONTFIX.

Comment 13

[Honza Bambas (:mayhemer)]

(In reply to Michal Novotny (:michal) from comment #12)
> Not sure what you exactly mean. If you believe that the code is called even
> when the old cache is disabled by the pref, then this could be easily
> verified by ignoring the pref. If the crash persists, we have a problem. If
> it's gone, then this is WONTFIX.

OK, file a bug!

Comment 14

[Honza Bambas (:mayhemer)]

This could be a duplicate of bug 1379353, since all of the crashes are happening on x64 and now I noticed they all very likely come from a single user :)  Hence, removing the pref will fix this.

Comment 15

[Shian-Yow Wu [:swu]]

Mark Firefox 56 tracking flag as wontfix based on comment 12, as it's no longer seen in 56.