David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Description • 8 years ago

This bug was filed from the Socorro interface and is 
report bp-677b86b4-3d94-4432-95bc-3a06d2160506.
=============================================================

A new topcrash appeared in Fennec nightly starting 2016-05-06.

The one day regression window is:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=369a5ee3a2880a4a98df3a00bf3db8d8f36b181b&tochange=25d777f7efb357fc5478251913548521986abaa0
which means the obvious candidate appears to be:
https://hg.mozilla.org/mozilla-central/rev/78731bdec697

Query for all the crashes on nightly at:
https://crash-stats.mozilla.com/signature/?product=FennecAndroid&release_channel=nightly&platform=Android&date=%3E%3D2016-04-15&signature=mozilla%3A%3Agl%3A%3ACreateSurfaceForWindow

	Nicholas Nethercote [inactive]
	Comment 1 • 8 years ago

Several distinct crash signatures all match this one, I think -- they all go through mozilla::gl::CreateSurfaceForWindow, and they all crash at address 0x58.

	Nicholas Nethercote [inactive]
	Comment 2 • 8 years ago

By my count there are 35 crashes with this signature in Nightly 20160506052037. That would normally easily be #1 topcrash on Fennec, though in this case it's #2 because bug 1271020 is even worse.

	Nicholas Nethercote [inactive]
	Comment 3 • 8 years ago

I also see a bunch more crash reports without stack traces that crash at address 0x58, and so may have the same root cause. The signatures involved include:

@0xb6749320
@0xb6754cb0
@0xb67b9320
@0xb6700cb0
@0xb6d49284
@0xf7034cb0
@0xf72e82b1

	Kan-Ru Chen [:kanru] (UTC+9)
	Comment 4 • 8 years ago

More crashes appeared under the signature [@ std::rethrow_exception]

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 5 • 8 years ago

A possible fix, though since I can't reproduce this I'm not sure this is the cause.

	(inactive) Jim Chen [:jchen] [:darchons]
	Comment 6 • 8 years ago

Bug 1271773 has STR that produces a crash report that's very similar to this bug. The STR is to install the ZXing Barcode Scanner and click the barcode icon in the Fennec URL bar to launch the scanner.

	Nick Thomas [:nthomas] (UTC+12)
	Comment 7 • 8 years ago

I took attachment 8751493 [details] [diff] [review] for a spin on try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ad4ef8a529c&selectedJob=20723609) and still hit an 0xb67b9320 crash
 https://crash-stats.mozilla.com/report/index/0df4f5af-b507-4831-aa0f-e04962160512

Most of my crashes (16 in total) are similarly useless but this one
 https://crash-stats.mozilla.com/report/index/c92d08ac-e032-40f0-beeb-ab48e2160507
looks a match to this bug.

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 8 • 8 years ago

Alright, thanks Nick. Back to the drawing board I guess.

	(inactive) Jim Chen [:jchen] [:darchons]
	Comment 9 • 8 years ago

One problem with the patch in bug 1136364 is that it changed the type id in nsWindow::GetNativeData from NS_NATIVE_NEW_EGL_SURFACE to NS_NATIVE_WINDOW. NS_NATIVE_WINDOW is used by a lot of unrelated code, and can be called from the main thread, compositor thread, etc. Using NS_NATIVE_WINDOW is inefficient because we're getting a surface unnecessarily. It can also easily cause race conditions when multiple threads try to call GetNativeData(NS_NATIVE_WINDOW) at the same time.

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 10 • 8 years ago

Some minor changes to give us more info about the crash. Carrying over snorp's r+.

	Pulsebot
	Comment 11 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e39f7fd724

	Pulsebot
	Comment 12 • 8 years ago

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

	Phil Ringnalda (:philor)
	Comment 13 • 8 years ago
bugherder

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

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 15 • 8 years ago

Sorry, I made a mistake with bzexport and exported the wrong changeset to the wrong bug.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 16 • 8 years ago

Still the #2 topcrash in fennec, with 147 crashes from 92 installs in the last week.

	Jamie Nicol [:jnicol]
	Comment 17 • 8 years ago

I have STR:

1) Close firefox
2) Open link from different app.
3) Just after firefox opens press home button to minimise app
4) The crash reporter should appear after a couple of seconds.

I'd guess that when the app is minimised the surface we get from AcquireNativeWindow() is no longer valid, causing eglCreateWindowSurface to crash?

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 18 • 8 years ago

I've just recently been able to somewhat reliably reproduce by running in an emulator on a Linux VM (at least, whenever I'm able to attach the debugger without getting an ElfLoader crash, I eventually hit this crash), and it doesn't require minimizing. I do think we're getting a stale surface somehow, but I'm still not able to figure out exactly what's going awry.

	Nicholas Nethercote [inactive]
	Comment 19 • 8 years ago

This is the #1 Fennec crash on Nightly by such a long way that we might as well ignore all other crashes until this one is fixed.

For example, in the crashes for Nightly 20160527030220:

https://crash-stats.mozilla.com/search/?product=FennecAndroid&build_id=20160527030220&platform=Android&date=%3E%3D2016-05-27&release_channel=nightly&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

I see 6 separate crash signatures that definitely map to this bug, 2 that definitely don't, and among a sampling of the remaining 42 (which are all just addresses like @0xb6805ce0) the majority look to be this bug too. (You can tell because the crash address is 0x58).

Margaret, I'm needinfo'ing you in case this isn't already on your radar.

	:Margaret Leibovic
	Comment 20 • 8 years ago

This is a platform issue, so snorp's team should handle it. It looks like Dylan already has a patch here, hopefully he can make more progress on that and land it soon.

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 21 • 8 years ago

This prevents LayerSurfaceView.onLayout from calling LayerView.surfaceChanged when we do not yet have a valid surface.

	Sebastian Kaspari (:sebastian; :pocmo)
	Comment 25 • 8 years ago

My bug 1277481 has been closed as a duplicate of this. On my HTC 9 this is happening every time I open settings and every time I pause and resume the app. It's basically unusable.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 26 • 8 years ago

Is this ready to land?

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 27 • 8 years ago

(ni? snorp as well in case droeh is away or something.)

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 28 • 8 years ago

Unfortunately no, snorp was able to reproduce this crash with the latest patch.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 29 • 8 years ago

Is it still viable to try backing out bug 1136364, which based on comment 0 seems to be the regressing bug?

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 30 • 8 years ago

Yes. If we can't come up with a last minute fix, that's pretty much the plan.

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 31 • 8 years ago

This backs out the patch for bug 1136364 and related patches, and should certainly fix the crash.

	Pulsebot
	Comment 32 • 8 years ago

Pushed by droeh@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dee200e969ee
Backs out the patch for Bug 1136364 and related patches. r=snorp

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 33 • 8 years ago

Comment on attachment 8760762 [details] [diff] [review]
Backout patch

Approval Request Comment
[Feature/regressing bug #]: 1136364
[User impact if declined]: Frequent crashes
[Describe test coverage new/current, TreeHerder]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9462e7f60687&selectedJob=22116066
[Risks and why]: Low-risk, this reverts changes that caused a major crash.
[String/UUID change made/needed]: N/A

	Nicholas Nethercote [inactive]
	Comment 34 • 8 years ago

Thank you for backing out. This was a bad one :(

	Carsten Book [:Tomcat]
	Comment 35 • 8 years ago
bugherder

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

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 36 • 8 years ago

snorp, want to also do the back out on 49 (now aurora)? The merge was on June 6th so this missed the cut off.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 37 • 8 years ago

Never mind! I just saw the approval request.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 38 • 8 years ago

Comment on attachment 8760762 [details] [diff] [review]
Backout patch

Backouts to fix a topcrash. Please uplift to aurora 49.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 39 • 8 years ago

Wes can you land this? Just want to make sure as I try to catch up with the backlog from the all hands meeting.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 40 • 8 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/364d6e860e97

Should this also resolve the bug or is there more to do here?

	Ryan VanderMeulen [:RyanVM]
	Comment 41 • 8 years ago

Presumably we want to confirm that the crash is gone first. I did go ahead and re-open bug 1136364 though.

	:gerard-majax
	Comment 42 • 8 years ago

(In reply to Ryan VanderMeulen [:RyanVM] from comment #41)
> Presumably we want to confirm that the crash is gone first. I did go ahead
> and re-open bug 1136364 though.

At least for me, since the backout landed on nightly, no problem anymore

	Dylan Roeh (:droeh) (he/him) Assignee
	Comment 43 • 8 years ago

Yeah, I think we can safely call this resolved. On Aurora/49 we've backed out the changes that were causing the crash, and on Nightly/50 we backed them out and replaced them with updated patches that should fix the crashes (see bug 1136364, bug 1280369, and bug 1280594).

	Firefox Bug Husbandry Bot
	Comment 47 • 6 years ago

Removing leave-open keyword from resolved bugs, per :sylvestre.