950888 - crash in ExperimentRunner::Run() at startup

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[u279076]

This bug was filed from the Socorro interface and is 
report bp-a2bb4fb8-7043-440e-aaae-10d612131215.
=============================================================
0 	xul.dll 	ExperimentRunner::Run() 	netwerk/dns/nsHostResolver.cpp
1 	xul.dll 	nsThread::ProcessNextEvent(bool,bool *) 	xpcom/threads/nsThread.cpp
2 	xul.dll 	nsThread::ThreadFunc(void *) 	xpcom/threads/nsThread.cpp
3 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c

URL Correlations:
22 	about:home
13 	about:blank
3 	https://addons.mozilla.org/en-US/developers/docs/how-to
3 	https://bugzilla.mozilla.org/enter_bug.cgi#h=dupes%7CMozilla+Localizations
2 	jar:file:///C:/Program%20Files%20(x86)/Aurora/browser/omni.ja!/chrome/browser...
1 	http://snf.bplaced.net/
1 	https://addons.mozilla.org/en-US/firefox/addon/blackfox-v1-blue/?src=search
1 	https://www.google.co.uk/search?q=firefox+beta&ie=utf-8&oe=utf-8&rls=org.mozi...
1 	http://www.mozilla.org/beta/
1 	about:addons
1 	javascript:'';
1 	https://www.google.com.au/
1 	http://www.ant.com/video-downloader/ff-installed
1 	http://www.firefox.com/
1 	https://bugzilla.mozilla.org/enter_bug.cgi
1 	https://a.gfx.ms/marketDateClass_en-sg_d7huULhSMX9M0tSh_JH3PA2.js
1 	https://addons.mozilla.org/en-US/firefox/addon/1-click-youtube-video-download/
1 	about:newaddon?id=[e-mail]
1 	http://www.mozilla.org/products/download.html?product=firefox-beta-stub&os=wi...
1 	http://www.mozilla.org/en-US/firefox/beta/
1 	about:newtab
1 	https://www.facebook.com/
1 	http://www.shanaproject.com/
1 	https://services.addons.mozilla.org/es-ES/firefox/discovery/pane/28.0a2/WINNT...

Platforms:
Windows 7 	63.01 %	46
Windows Vista 	15.07 %	11
Windows 8 	12.33 %	9
Windows 8.1 	5.48 %	4
Windows XP 	4.11 %	3 

Ranking:
#3 on Aurora @ 3.54%
#11 on Nightly @ 1.08%
N/A on Beta

I don't see any interesting comments in the reports so I don't have much to go on for testing. I'll check to see when this first showed up and check the pushlogs.

Comment 1

[u279076]

Looks like this first showed up in crashstats with Firefox 28 on 2013-12-07 which would implicate the following pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1401e4b394ad&tochange=42b2a2adda8f

Unfortunately without a better understanding of the steps to reproduce and/or what code this signature represents it will be hard for me to break this down further.

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

Gavin - is there someone who can look at that pushlog and try to determine potential cause?

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking since this is a topcrash.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Looks like Necko code (haven't had a chance to look through the pushlog, really wish  we had an "all changed files" view for pushlog like bonsai used to have...)

Comment 5

[Lukas Blakk [:lsblakk] use ?needinfo]

Patrick can you investigate further into the pushlog & changes made between 27-28 that might be causing this topcrash?

Comment 6

[Benjamin Smedberg]

This is clearly 	c0128d154ed3	Nicholas Hurley — Bug 942317 - An experiment to test how reachable SRV records are in the wild. r=mcmanus

Comment 7

[u408661]

Attachment: null check

I'm not convinced a null check is the problem here (the crashing line seems to be sometime after we would've already dereferenced a null pointer), but stranger things have happened (especially in win32-land), and I'm otherwise stumped so far, so this is definitely worth a shot.

Comment 8

[Patrick McManus [:mcmanus]]

I'm never convinced about any crash report until it goes away :)

the crash is a deref of 0x18 somewhere in this conditional..

	if (srvStatus == DNS_RCODE_NOERROR) {
            DNS_SRV_DATAA *srvData = &srvResults->Data.Srv;
            if (_stricmp(srvData->pNameTarget, "success.http2test.mozilla.org") ||
		srvData->wPort != 443 ||
                srvData->wPriority != 100 ||
                srvData->wWeight != 100) {
                correctSRV = false;
	    }

so it makes sense to me the compiler has pretty much just inlined the srvData calculation into that first stricmp() call (probably reusing it later).. 

of course that's not consistent with getting a NOERROR status code - or it doesn't seem that way :(

Comment 9

[Patrick McManus [:mcmanus]]

bake it to see if it helps the crash stats.. then we can backout/uplift depending on the result

Comment 10

[u408661]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=4f1cad998c80

Comment 11

[u408661]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e5def2cce4bc

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 13

[u279076]

This is currently the #4 crash on Firefox Aurora 28 at 2.33%. However, on Firefox Nightly 29 this is currently #128 at 0.11% and has dropped 64 positions in the last week.

Comment 14

[u279076]

Current Rank:
 * Firefox 29: N/A (2 crashes)
 * Firefox 28: #13 @ 0.83% (-1.28%)
 * Firefox 27: N/A (0 crashes)
 * Firefox 26: N/A (0 crashes)

This has dropped significantly on previously affected branches. Can we call this fixed?

Comment 15

[Patrick McManus [:mcmanus]]

didn't mean to make those last changes to this bug (but leaving fixed-29 cause that's true)

waiting for backport to aurora to close this

Comment 16

[Patrick McManus [:mcmanus]]

(In reply to Patrick McManus [:mcmanus] from comment #15)

> waiting for backport to aurora to close this

actually 957759 backed the offending code out of ff28/aurora so its ok imo to close

Comment 17

[Patrick McManus [:mcmanus]]

I clearly need to change to espresso.

957759 will fix this on aurora, but it has only been a+'d for that channel and not landed yet.

Comment 18

[Robert Kaiser]

(In reply to Patrick McManus [:mcmanus] from comment #17)
> 957759 will fix this on aurora, but it has only been a+'d for that channel
> and not landed yet.

Well, if it's a backout of the functionality that caused this, it's probably best to mark this "disabled" instead of "fixed" for 28 once it does, I guess.

Comment 19

[:Gavin Sharp [email: [e-mail]]]

(In reply to Robert Kaiser (:[e-mail]) from comment #18)
> Well, if it's a backout of the functionality that caused this, it's probably
> best to mark this "disabled" instead of "fixed" for 28 once it does, I guess.

"disabled" is for the bug that got backed out. This bug is "fixed" by the disabling.

Comment 20

[u279076]

This has dropped to #36 on Aurora with 38 reports and only has one report on Nightly in the last week. In addition, there are no reports with build IDs corresponding to dates following the landing of the backout (the last buildID reported is 2014011600). As such, I'm calling this verified fixed.