Greg Grisco Reporter
	Description • 10 years ago

[Blocking Requested - why for this release]:

Same as bug 1038900 which was marked fixed, but crash is still happening.

	Greg Grisco Reporter
	Comment 3 • 10 years ago

Hi Jed, can you take a look at this one?

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 4 • 10 years ago

This is an entirely different crash from bug 1038900, and I think I know what's going on.  These messages:

07-30 08:12:49.546  3510  3515 E Sandbox : SANDBOX DISABLED FOR DMD!  See bug 956961.
[...]
07-30 08:12:49.566  3416  3416 E Sandbox : SANDBOX DISABLED FOR DMD!  See bug 956961.

are being logged from an asynchronous signal handler, with logging code that isn't async signal safe.  This wasn't as much of an issue for the SIGSYS handler, because at that point we're already about to crash and things can't get much worse — but this is not that.  I conjecture that this caused a deadlock due to reentrant use of malloc, and that this is why these two messages occur back-to-back (note the thread ids):

07-30 08:13:00.646  3432  3432 E Sandbox : Thread 3495 unresponsive for 10 seconds.  Killing process.
07-30 08:13:00.646  3432  3495 E Sandbox : SANDBOX DISABLED FOR DMD!  See bug 956961.

This should affect DMD builds only.  Also, bug 956961 removed that code so 2.1 is unaffected (and any fix for this will need to be landed directly on 2.0).

The fix is simple: do the DMD check once ahead of time instead of in the routine that runs once for each thread.

	cafbot (PoC: ggrisco)
	Comment 5 • 10 years ago

Observed on: 

Device: msm8610
Gonk Version: AU_LINUX_GECKO_B2G_KK_3.6.01.04.00.000.043
Moz BuildID: 20140721000201
B2G Version: 2.0
Gecko Version: 32.0a2
Gaia:  http://git.mozilla.org/?p=releases/gaia.git;a=commit;h=8cb1a949f2e9650bb2c5598e78a6f24a58bbaf97
Gecko: http://git.mozilla.org/?p=releases/gecko.git;a=commit;h=5f27d3ee3ccf01ac91a3efacb5e3e22ea62fd73c

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 6 • 10 years ago

I moved the other getenv call as well — it might not be part of the problem, but it also doesn't belong there.

	Guillaume Destuynder [:kang] [this account is no longer active]
	Comment 7 • 10 years ago

Comment on attachment 8465118 [details] [diff] [review]
Lift sandbox disablement checks out of async signal context.

Review of attachment 8465118 [details] [diff] [review]:
-----------------------------------------------------------------

That reasoning sounds right to me, too. Code looks good.

	Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ Assignee
	Comment 8 • 10 years ago

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/85ec57090a25

	Yeojin Chung [:YeojinC]
	Comment 9 • 10 years ago

No STR is present to create test case to address bug.