1083425 - Crash when searching on expansys.ca

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Hubert Figuiere [:hub]]

Go to expansys.ca 

Start searching. You might need to start over, refine, etc. 

Firefox crashed.

Here is the crash stack trace:

Program received signal SIGSEGV, Segmentation fault.
nsAString_internal::EndReading (this=this@entry=0x60, aIter=...) at /home/hub/source/mozilla/src/xpcom/string/nsTSubstring.h:125
125	    aIter.mStart = mData;
Missing separate debuginfos, use: debuginfo-install gstreamer-plugins-good-0.10.31-10.fc20.x86_64 gstreamer-plugins-ugly-0.10.19-14.fc20.x86_64 libmad-0.15.1b-16.fc19.x86_64
(gdb) where
#0  nsAString_internal::EndReading (this=this@entry=0x60, aIter=...) at /home/hub/source/mozilla/src/xpcom/string/nsTSubstring.h:125
#1  0x00007ffff2e07a23 in AppendUTF16toUTF8 (aSource=..., aDest=...) at /home/hub/source/mozilla/src/xpcom/string/nsReadableUtils.cpp:158
#2  0x00007ffff2e07af4 in AppendUTF16toUTF8 (aSource=..., aDest=...) at /home/hub/source/mozilla/src/xpcom/string/nsReadableUtils.cpp:146
#3  0x00007ffff3c64b53 in mozilla::dom::WebSocketImpl::GetName (this=<optimized out>, aName=...) at /home/hub/source/mozilla/src/content/base/src/WebSocket.cpp:2173
#4  0x00007ffff2ea22a4 in nsLoadGroup::Cancel (this=0x7fffa2e029c0, status=NS_BINDING_ABORTED) at /home/hub/source/mozilla/src/netwerk/base/src/nsLoadGroup.cpp:280
#5  0x00007ffff33abfba in nsDocLoader::Stop (this=this@entry=0x7fffa2e8f000) at /home/hub/source/mozilla/src/uriloader/base/nsDocLoader.cpp:262
#6  0x00007ffff4234a9e in nsDocShell::Stop (this=this@entry=0x7fffa2e8f000) at /home/hub/source/mozilla/src/docshell/base/nsDocShell.h:184
#7  0x00007ffff42272a5 in nsDocShell::Stop (this=this@entry=0x7fffa2e8f000, aStopFlags=1) at /home/hub/source/mozilla/src/docshell/base/nsDocShell.cpp:5316
#8  0x00007ffff422cdd8 in nsDocShell::InternalLoad (this=this@entry=0x7fffa2e8f000, aURI=0x7fffa4d238e0, aReferrer=0x7fff8fe66ac0, aOwner=<optimized out>, aFlags=aFlags@entry=0, 
    aWindowTarget=<optimized out>, aTypeHint=0x7ffff4fb9950 <gNullChar> "", aFileName=..., aPostData=0x0, aHeadersData=0x0, aLoadType=2097153, aSHEntry=0x0, aFirstParty=true, aSrcdoc=..., 
    aSourceDocShell=0x7fffa2e8f1a0, aBaseURI=0x0, aDocShell=0x0, aRequest=0x0) at /home/hub/source/mozilla/src/docshell/base/nsDocShell.cpp:9974
#9  0x00007ffff4231838 in nsDocShell::OnLinkClickSync (this=0x7fffa2e8f000, aContent=0x7fff921537a0, aURI=0x7fffa4d254e0, aTargetSpec=<optimized out>, aFileName=..., aPostDataStream=0x0, 
    aHeadersDataStream=0x0, aDocShell=0x0, aRequest=0x0) at /home/hub/source/mozilla/src/docshell/base/nsDocShell.cpp:13179
#10 0x00007ffff4234e0a in OnLinkClickEvent::Run (this=0x7fffa33d6e60) at /home/hub/source/mozilla/src/docshell/base/nsDocShell.cpp:12964
#11 0x00007ffff2e45b69 in nsThread::ProcessNextEvent (this=0x7ffff7dd9cc0, aMayWait=<optimized out>, aResult=0x7fffffffc5af) at /home/hub/source/mozilla/src/xpcom/threads/nsThread.cpp:830
#12 0x00007ffff2e5d650 in NS_ProcessNextEvent (aThread=<optimized out>, aMayWait=<optimized out>) at /home/hub/source/mozilla/src/xpcom/glue/nsThreadUtils.cpp:265
#13 0x00007ffff3042e97 in mozilla::ipc::MessagePump::Run (this=0x7fffeb47adc0, aDelegate=0x7ffff7d8c6a0) at /home/hub/source/mozilla/src/ipc/glue/MessagePump.cpp:99
#14 0x00007ffff302e210 in RunHandler (this=0x7ffff7d8c6a0) at /home/hub/source/mozilla/src/ipc/chromium/src/base/message_loop.cc:223
#15 MessageLoop::Run (this=0x7ffff7d8c6a0) at /home/hub/source/mozilla/src/ipc/chromium/src/base/message_loop.cc:197
#16 0x00007ffff3bfc137 in nsBaseAppShell::Run (this=0x7fffe5e21240) at /home/hub/source/mozilla/src/widget/xpwidgets/nsBaseAppShell.cpp:164
#17 0x00007ffff437d65d in nsAppStartup::Run (this=0x7fffe5e46100) at /home/hub/source/mozilla/src/toolkit/components/startup/nsAppStartup.cpp:280
#18 0x00007ffff43add1a in XREMain::XRE_mainRun (this=this@entry=0x7fffffffc850) at /home/hub/source/mozilla/src/toolkit/xre/nsAppRunner.cpp:4165
#19 0x00007ffff43adf8a in XREMain::XRE_main (this=this@entry=0x7fffffffc850, argc=argc@entry=1, argv=argv@entry=0x7fffffffdcd8, aAppData=aAppData@entry=0x7fffffffca50)
    at /home/hub/source/mozilla/src/toolkit/xre/nsAppRunner.cpp:4238
#20 0x00007ffff43ae1cf in XRE_main (argc=1, argv=0x7fffffffdcd8, aAppData=0x7fffffffca50, aFlags=<optimized out>) at /home/hub/source/mozilla/src/toolkit/xre/nsAppRunner.cpp:4452
#21 0x0000000000404188 in do_main (argc=argc@entry=1, argv=argv@entry=0x7fffffffdcd8, xreDirectory=0x7ffff7d52780) at /home/hub/source/mozilla/src/browser/app/nsBrowserApp.cpp:287
#22 0x00000000004044dc in main (argc=1, argv=0x7fffffffdcd8) at /home/hub/source/mozilla/src/browser/app/nsBrowserApp.cpp:652


This is m-i, @ db0912d0106d
Linux F20 - x86_64
(I built it)

Comment 1

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]: Crashes

Andrea, is this fallout from websocket-on-workers?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Also, Hubert, if you still have this in a debugger, where is the actual crash happening?  What is the actual value of mData and other variables (like aIter)?

Comment 3

[Hubert Figuiere [:hub]]

I got out of the debugger: this is my main browser. I need it.

I'll see if I can reproduce again (did it twice before actually debugging).

Comment 4

[Hubert Figuiere [:hub]]

Program received signal SIGSEGV, Segmentation fault.
nsAString_internal::EndReading (this=this@entry=0x60, aIter=...) at /home/hub/source/mozilla/src/xpcom/string/nsTSubstring.h:125
125	    aIter.mStart = mData;
(gdb) p mData
Cannot access memory at address 0x60
(gdb) p aIter
$1 = (nsAString_internal::const_iterator &) @0x7fffcb362fc8: {mStart = 0x7fffcb363080 u"よ쬶翿", mEnd = 0x7fffcb363000 u"む쬶翿", 
  mPosition = 0x7f585ca0773c <nsACString_internal::SetCapacity(unsigned int)+20> u"삄ݵ\xdf89⧨;䠀쒃嬘썝䡕呁襁叴襈\xffc8\xffff襄\x863䅛嵜郃䡕呁襁䣴喍可襈䣻ﻺ\xffff삄Ѵ襄\x863奚䅛嵜郃䡕䡓ﮉ荈\x18ecﺃ瓿㬅\x877≵䎋ꠌ琘뀄ꠣ琄䠐\x38b赈ߨ1萀瓀诨\x873赈襈ﾒ\xffff荈\x18c4嵛嗃襈句譒\x85f\x9fe8\062䠀쌁㥈瓘訔贐齊呂眙茅⃪ႈｈ壧嵛嗃襈句譒\x85f燨2䠀쌁㥈瓘訔贐뽊呂眙茅\x20c2ႈｈ壧嵛郃ㅕ䣀啁呁襁叔襈击㭄\x867\x2073襁菵ￎ㗨\xffff蓿痀謈\x87bר:䠀\x38b衆ư孚屁嵁썝䡕坁噁啁呁襉菴ￎ䡓ﮉ荈⣬裡\xfffe蓿痀謈\x87b짨9䰀\x2b8b蕍䓤王䴈橴蕍瓭䕥恴쀱荈\xffc9襌䶮"...}
(gdb) k


It took me longer to reproduce. Let me know what else you need.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hubert, that's perfect, thanks.

So the crashing line in GetName is this:

2173   CopyUTF16toUTF8(mWebSocket->mOriginalURL, aName);

and I bet mWebSocket is null, which is why "this" ends up near-null when we invoke methods on mWebSocket->mOriginalURL.

Definitely smells like a regression from bug 504553.

Comment 6

[Andrea Marchesini [:baku]]

Attachment: crash2.patch

So, I don't know if this is the correct way to fix this issue.
First of all because I cannot reproduce it and then because in theory:

1. mWebSocket is null only when the WS is fully disconnected
https://mxr.mozilla.org/mozilla-central/source/content/base/src/WebSocket.cpp#521

2. and at that point we've already removed the nsIRequest from the loadGroup:
https://mxr.mozilla.org/mozilla-central/source/content/base/src/WebSocket.cpp#534

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8506121 [details] [diff] [review]
crash2.patch

Those asserts look useless. We crash anyway when accessing null mWebSocket.


> NS_IMETHODIMP
> WebSocketImpl::GetName(nsACString& aName)
> {
>   AssertIsOnMainThread();
> 
>+  if (!mWebSocket || mWebSocket->ReadyState() == WebSocket::CLOSED) {

Why the ReadyState() check?


> WebSocketImpl::IsPending(bool* aValue)
> {
>   AssertIsOnTargetThread();
> 
>+  if (!mWebSocket || mWebSocket->ReadyState() == WebSocket::CLOSED) {
>+    return NS_ERROR_FAILURE;
>+  }
>+
>   int64_t readyState = mWebSocket->ReadyState();
>   *aValue = (readyState != WebSocket::CLOSED);
This looks odd. We return early if ReadyState is CLOSED, and then here compare to CLOSED.
Why the ReadyState() need to be checked in the 'if'


> WebSocketImpl::GetLoadGroup(nsILoadGroup** aLoadGroup)
> {
>   AssertIsOnMainThread();
>+  MOZ_ASSERT(mWebSocket);
> 
>   *aLoadGroup = nullptr;
Just null check here, after setting *aLoadGroup to null.

Comment 8

[Andrea Marchesini [:baku]]

Attachment: crash2.patch

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8506192 [details] [diff] [review]
crash2.patch

Crossing fingers the analysis was right :)

Comment 10

[Andrea Marchesini [:baku]]

remote:   https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=555fe5ff434b

waiting for green.

Comment 11

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4a51a12d8ed3

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4a51a12d8ed3

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

We need this on Aurora, right?

Comment 14

[Andrea Marchesini [:baku]]

Comment on attachment 8506192 [details] [diff] [review]
crash2.patch

Approval Request Comment
[Feature/regressing bug #]: bug 504553
[User impact if declined]: a crash
[Describe test coverage new/current, TBPL]: I was not able to reproduce the issue. So we don't have a particular mochitest for this.
[Risks and why]: The patch is easy enough.
[String/UUID change made/needed]: none.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f84bc950b7b9

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Did the crashes go away?

Comment 19

[u279076]

(In reply to Olli Pettay [:smaug] from comment #18)
> Did the crashes go away?

It's seems like this is gone. There have been 0 crashes reported with a Fennec builds with a build date of 2014-10-22 or later. Marking this verified fixed but we may want to keep an eye on this just to be sure.

Comment 20

[Hubert Figuiere [:hub]]

couldn't get it to crash either.

Thank you kindly.