814549 - nsAppShell destructor segfaults when no input reader thread is created (as happens in an OOP app)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

Attachment: backtrace

Device: otoro

gaia: git://github.com/mozilla-b2g/gaia
gaia: e1448496e18fd51aa79429617af99639e94828cb

gecko: git://github.com/mozilla/releases-mozilla-central
gecko: e5d9537ac01ac0da5dbb8cc73e2e2c7a80a30d46

I was using a debug build.

I happened to be testing the same app when run in-process and it doesn't segfault when run in-process.

STR:
- Launch App (I tried Music and Calculator)
- Once the app has finished displaying, press the HOME button
- Back at the home screen, long press the home button. This brings up the thumbnail of whichever app you launched
- swipe up to terminate the app.

While terminating, the app will crash. This seems to be very close to the end of normal termination.

It hits a segfault in nsAppShell destructor due to mReadThread being NULL.

backtrace and logcat attached.

Comment 1

[Dave Hylands [:dhylands]]

Attachment: logcat

Comment 2

[Dave Hylands [:dhylands]]

This bug has been around for a while, since it also happens in my other tree, which is currently based around Nov 9. The above backtrace was from a Nov 22 tree.

Comment 3

[Dave Hylands [:dhylands]]

Attachment: Patch to fix null mReaderThread dereference

Comment 4

[Dave Hylands [:dhylands]]

After fixing the null mReaderThread, I now get the following:

Program received signal SIGSEGV, Segmentation fault.
android::SharedBuffer::release (this=0xfffffff0, flags=0) at frameworks/base/include/utils/SharedBuffer.h:139
139	    return (mRefs == 1);
(gdb) bt
#0  android::SharedBuffer::release (this=0xfffffff0, flags=0) at frameworks/base/include/utils/SharedBuffer.h:139
#1  0x422490ac in android::terminate_string16 () at frameworks/base/libs/utils/String16.cpp:55
#2  0x422483a8 in ~LibUtilsFirstStatics (this=0xfffffff0, __in_chrg=<value optimized out>) at frameworks/base/libs/utils/Static.cpp:38
#3  0x4010bc34 in __cxa_finalize (dso=0x0) at bionic/libc/stdlib/atexit.c:158
#4  0x4010bfd0 in exit (status=0) at bionic/libc/stdlib/exit.c:57
#5  0x4010bfd0 in exit (status=0) at bionic/libc/stdlib/exit.c:57
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

which looks like a pointer with a negative index.

Comment 5

[Dave Hylands [:dhylands]]

I filed bug 814783 to cover the segfault in comment 4

Comment 6

[Dave Hylands [:dhylands]]

Attachment: Patch to fix null mReaderThread dereference - v2

Comment 7

[Dave Hylands [:dhylands]]

Related bugs (which also cause segfaults at app close time) are bug 814769 and bug 814783

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 684804 [details] [diff] [review]
Patch to fix null mReaderThread dereference - v2

Oy, this code is so scary.  Bug 783750 can't come soon enough :/.

Comment 9

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11d3d401116b

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/11d3d401116b

Comment 11

[Dave Hylands [:dhylands]]

When I discovered this bug, the crash in bug 816671 was one of the segfaults I also encountered as a related problem. At the time I was only seeing the crash in debug builds, so I didn't nom it. But since we are in fact seeing the crash in bug 816671, I'm noming this one.

There should be no risk associated with taking this patch for non-B2G platforms. The if statement that was introduced only prevents a segfault. Right now the only conditions for the segfault occuring are on B2G child content processes.

Comment 12

[Andreas Gal :gal]

Why should this block? I think you want an approval here.

Comment 13

[Andreas Gal :gal]

a=me

Comment 14

[Dave Hylands [:dhylands]]

Comment on attachment 684804 [details] [diff] [review]
Patch to fix null mReaderThread dereference - v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Crash reports
Testing completed (on m-c, etc.): https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=11d3d401116b
Risk to taking this patch (and alternatives if risky): almost none. The fix only prevents a segfault. So far the segfault is only observed in content children running as part of a debug build, but related crash from bug 816671 is now being observed on unagi devices.
String or UUID changes made by this patch: None

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4e1ff094f22b
https://hg.mozilla.org/releases/mozilla-beta/rev/25ec34410885