81764 - Cache related crash [@ nsCacheEntry::GetData]

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[R.K.Aa.]

Loading http://norsk.lysingsblad.no crashes moz on Linux during load.

I commented in bug 72507 which was the only relevant bug i found - i pulled not
long after the fix there was checked in and found i crashed. (RH7.1/gcc 2.96)
Filing as separate bug since noone has commented yet.

Downloaded nightlies to compare against:
Build ID 2001051813 Linux did not have the crash.
Build ID 2001051821 has it.

Have deleted cache inbetween tests: The crash is 100% reproducable here.

Comment 1

[R.K.Aa.]

Attachment: backtrace

Comment 2

[R.K.Aa.]

FWIW: Rebuilt with bryner's v1.593 of nsCSSFrameConstructor.cpp
("Views aren't refcounted!")  but it still crashes.

Comment 3

[Daniel Bratell]

I saw a crash with that stack trace on Win2000 too so I set platform to all. I 
don't crash on the http://norsk.lysingsblad.no site though. 

This was with a build from 2001-05-19 ~03:00 PDT.

Comment 4

[R.K.Aa.]

getting same backtrace by going to http://www.lokigames.com
Click "support"
Console says page loaded successfully, but nothing really happened.
Click the link once more 
Crash

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I just saw this clicking the "Magazine" link on http://nytimes.com/ (on the left
side of the page).

The immediate cause of the crash was that entry is null in
nsDiskCacheDevice::FindEntry, so the null is passed to CreateBinding:


#3  0x402e207e in pthread_sighandler (signo=11, ctx=
      {gs = 0, __gsh = 0, fs = 0, __fsh = 0, es = 43, __esh = 0, ds = 43, __dsh
= 0, edi = 1082943228, esi = 3221209448, ebp = 3221209344, esp = 3221209344, ebx
= 1082944028, edx = 0, ecx = 0, eax = 52, trapno = 14, err = 4, eip =
1082889190, cs = 35, __csh = 0, eflags = 2163350, esp_at_signal = 3221209344, ss
= 43, __ssh = 0, fpstate = 0xbfffbe80, oldmask = 2147483648, cr2 = 52}) at
signals.c:97
	self = 0x402eaf60
	in_sighandler = 0x0
	self = 0x402eaf60
	in_sighandler = 0x0
#4  <signal handler called>
No locals.
#5  0x408b93e6 in nsCOMPtr<nsISupports>::get() const (this=0x34)
    at ../../../dist/include/nsCOMPtr.h:903
	this = (nsCOMPtr<nsISupports> *) 0xfffffffc
#6  0x408b90cd in nsCOMPtr<nsISupports>::operator nsDerivedSafe<nsISupports>*() 
const (this=0x34) at ../../../dist/include/nsCOMPtr.h:915
No locals.
#7  0x408a6c89 in nsCacheEntry::GetData(nsISupports**) (this=0x0, 
    result=0xbfffc168)
    at /builds/seamonkey/mozilla/netwerk/cache/src/nsCacheEntry.cpp:152
No locals.
#8  0x408af0eb in nsDiskCacheBindery::CreateBinding(nsCacheEntry*,
nsDiskCacheRecord*) (this=0x85cba28, entry=0x0, record=0xbfffc1a8)
    at /builds/seamonkey/mozilla/netwerk/cache/src/nsDiskCacheBinding.cpp:180
	data = {<nsCOMPtr_base> = {mRawPtr = 0x0}, <No data fields>}
	rv = 1081079346
	binding = (nsDiskCacheBinding *) 0x4073357c
#9  0x408b2996 in nsDiskCacheDevice::FindEntry(nsCString*) (this=0x85cba18, 
    key=0x43794868)
    at /builds/seamonkey/mozilla/netwerk/cache/src/nsDiskCacheDevice.cpp:625
	rv = 0
	record = {mHashNumber = 861620360, mEvictionRank = 3304576314, 
  mDataLocation = 2147485441, mMetaLocation = 2432696368}
	entry = (nsCacheEntry *) 0x0
	binding = (nsDiskCacheBinding *) 0x0
	hashNumber = 861620360
	diskEntry = (nsDiskCacheEntry *) 0x43794d68

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The reason it's null is that the |nsCRT::strcmp| shows that the strings are
different.  In this case, 

diskEntry->mKeyStart is
"HTTP:http://graphics.nytimes.com/images/2001/05/20/weekinreview/20mag-cover.1.gif"

and key->get() is
"HTTP:http://graphics.nytimes.com/images/2001/05/06/weekinreview/06mag-cover.1.gif"

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch that fixes crash, although I'm not sure if it's the right way to handle collisions

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

BTW, it might be nice to use a hash algorithm where it's not trivial to modify a
string and get the same hash number.  (Note that the above two strings have the
same change made 16 characters apart -- we get the same hash code if we change
any two bits separated by any multiple of 8 bytes.)

Comment 9

[gordon]

The patch looks reasonable, though I haven't tested it yet.  Cc'ing brendan, scc, 
and Darin concerning the hash function observation.  Scc has done some analysis 
that shows this hash function is pretty good in general.  I will add a couple of 
lines to keep some statistics on the frequency of this hash number collision in 
normal use (at least in debug builds).

r=gordon, or I can check this in later tonight or tomorrow after getting an sr.

Comment 10

[Matthias Versen [:Matti]]

*** Bug 81899 has been marked as a duplicate of this bug. ***

Comment 11

[R.K.Aa.]

Built with the patch - it fixes the crash.
Tested on the original site + lokigames

Since this bug surfaced i've had approx 10 minutes between crashes. Even less
under normal usage, when i don't "hang around" in bugzilla/bonsai
(the latter not being good testcases for this bug, since they are "?" sites not
taxing cache much)

Comment 12

[R.K.Aa.]

morph goddess in action..

Something i think surfaced after the checkin for bug 72507:
I suddenly have to click "submit" button in bugzilla queries repeatedly to make
it trigger. Before i patched with dbaron's patch here, the first click took me
nowhere, and the second click would make me crash with the backtrace frmo this bug.

After patching i can click..and click..and click.. before anything actually
starts loading. The throbber seems to attempt to "ignite" - and then dies again.

Similar things happen on other sites. Often i only see a blank page when opening
bug in new window. Have to reload repeatedly to make it load. This too caused
the above crash before i patched.

Smells related, and seems to rhyme with dbaron's comments.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

gordon:  I'd prefer if you dealt with this, since I'm not really sure of the
right way to fix this.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I just saw my build hang (100% CPU usage) in nsDiskCacheBindery::AddBinding.  It
might be related to this patch -- I'm not sure.  It seems like there isn't a
normal exit from the |while(1)|, only a pair of |return NS_ERROR_UNEXPECTED|. 
Should there be a |break;| somewhere?  (I suspect it may be related since it
looks like that code might be hit only in the case of a hash number collision.)

Comment 15

[R.K.Aa.]

These appeared at the same time:
Cousin-bug: bug 81799.
Crashes:
bug 81765, bug 81834
Reports of 100% cpu and hangs:
bug 81837, bug 81844, bug 81885

Comment 16

[gordon]

*** Bug 81849 has been marked as a duplicate of this bug. ***

Comment 17

[gordon]

Thanks for spotting that problem in AddBinding, David.  I'll get a fix ready for 
when the tree opens.

Comment 18

[Brendan Eich [:brendan]]

gordon: is the disk cache map using PLDHashNumbers as if they were perfect hash
function outputs?  They aren't.  Collisions will happen infrequently but surely.

/be

Comment 19

[gordon]

Fix has been checked in.

Brendan, we are not looking for a perfect hash, but we would like collisions to 
be rare.  We will probably need to modify our function to accomodate cases like 
the NYTimes.

Comment 20

[Jay Patel [:jay]]

adding topcrash keyword.  i was able to crash with build 2001052111.  i will 
try to reproduce with today's build to see if i can verify the fix.  Here is the 
crash info for future reference:

Incident ID 30735243 
 Trigger Time 
                2001-05-21 16:55:41 
 Email Address 
                [e-mail] 
 User Comments 
                just opened this page from a link in bug 70555. i was looking 
for the realplayer link shrirang was talking about, but
                before i could find anythign, it crashed. 
 Build ID
                2001052111 
 Product ID
                Netscape6.50 
 Platform ID
                Win32 
 Stack Trace

nsCacheEntry::GetData 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheEntry.cpp, line 152] 
nsDiskCacheBindery::CreateBinding 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheBinding.cpp, line 181] 
nsDiskCacheDevice::FindEntry 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsDiskCacheDevice.cpp, line 626] 
nsCacheService::SearchCacheDevices 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 758] 
nsCacheService::ActivateEntry 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 673] 
nsCacheService::ProcessRequest 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 571] 
nsCacheService::OpenCacheEntry 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheService.cpp, line 649] 
nsCacheSession::OpenCacheEntry 
[d:\builds\seamonkey\mozilla\netwerk\cache\src\nsCacheSession.cpp, line 82] 
nsHttpChannel::OpenCacheEntry 
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line 
550] 
nsHttpChannel::Connect 
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line 
203] 
nsHttpChannel::AsyncOpen 
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line 
1778] 
imgLoader::LoadImage 
[d:\builds\seamonkey\mozilla\modules\libpr0n\src\imgLoader.cpp, line 180] 
nsImageFrame::LoadImage 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsImageFrame.cpp, line 1379] 
nsImageFrame::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsImageFrame.cpp, line 1255] 
nsCSSFrameConstructor::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp, 
line 10023] 
StyleSetImpl::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp, line 1290] 
PresShell::AttributeChanged 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 4816] 
nsDocument::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\base\src\nsDocument.cpp, line 1676] 
nsHTMLDocument::AttributeChanged 
[d:\builds\seamonkey\mozilla\content\html\document\src\nsHTMLDocument.cpp, line 
1290] 
nsGenericHTMLElement::SetAttribute 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp, 
line 1473] 
nsHTMLImageElement::SetSrcInner 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLImageElement.cpp, 
line 935] 
nsHTMLImageElement::SetSrc 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLImageElement.cpp, 
line 1049] 
XPTC_InvokeByIndex 
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp, 
line 139] 
XPCWrappedNative::CallMethod 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp, line 
1837] 
XPC_WN_GetterSetter 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp, 
line 1266] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 809] 
js_InternalInvoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 897] 
js_SetProperty [d:\builds\seamonkey\mozilla\js\src\jsobj.c, line 2550] 
js_Interpret [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 2549] 
js_Invoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 825] 
js_InternalInvoke [d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 897] 
JS_CallFunctionValue [d:\builds\seamonkey\mozilla\js\src\jsapi.c, line 3309] 
nsJSContext::CallEventHandler 
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp, line 937] 
nsJSEventListener::HandleEvent 
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp, line 140] 
nsEventListenerManager::HandleEventSubType 
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp, line 
1120] 
nsEventListenerManager::HandleEvent 
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp, line 
1288] 
nsGenericElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp, line 1674] 
nsGenericHTMLElement::HandleDOMEventForAnchors 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsGenericHTMLElement.cpp,
line 1148] 
nsHTMLLinkElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLLinkElement.cpp, 
line 287] 
nsGenericElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\base\src\nsGenericElement.cpp, line 1696] 
nsHTMLImageElement::HandleDOMEvent 
[d:\builds\seamonkey\mozilla\content\html\content\src\nsHTMLImageElement.cpp, 
line 613] 
nsEventStateManager::GenerateMouseEnterExit 
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp, line 
2081] 
nsEventStateManager::PreHandleEvent 
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp, line 
340] 
PresShell::HandleEventInternal 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5509] 
PresShell::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5441] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 377] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 350] 
nsView::HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 350] 
nsViewManager::DispatchEvent 
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 2056] 
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 68] 
nsWindow::DispatchEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 706] 
USER32.dll + 0x1a43f (0x77e8a43f) 
nsWindow::DispatchMouseEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 4053] 
ChildWindow::DispatchMouseEvent 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 4298] 
nsWindow::ProcessMessage 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3051] 
nsWindow::WindowProc 
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 958] 
USER32.dll + 0x1820 (0x77e71820) 

Comment 21

[Tom Everingham]

verified:  Linux and winNT 6/5/01