855453 - B2G Desktop crashes on shutdown in RIL call to UnixSocket::Connect

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Repro:

- Bring up FxOS Desktop with ril compiled in
- Shut down

Expected:

Clean shutdown

Actual:

Crashes with the following stack in the IOThread:

#0  0x00007f389aad483d in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f389aad46dc in sleep () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007f38968ccf12 in ah_crap_handler (signum=11) at /home/qdot/code/mozbuild/mozilla-central/toolkit/xre/nsSigHandlers.cpp:88
#3  0x00007f38968d14ea in nsProfileLock::FatalSignalHandler (signo=11, info=0x7f388a6fa770, context=<optimized out>) at /home/qdot/code/mozbuild/mozilla-central/obj-debug/toolkit/profile/nsProfileLock.cpp:190
#4  <signal handler called>
#5  0x00007f3897b7e004 in mozilla::ipc::UnixSocketImpl::Connect (this=0x7f387a299f30) at /home/qdot/code/mozbuild/mozilla-central/ipc/unixsocket/UnixSocket.cpp:499
#6  0x00007f3897bfba13 in MessageLoop::RunTask (this=0x7f388a6facd8, task=0x7f38783fd660) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:334
#7  0x00007f3897bfd131 in MessageLoop::DeferOrRunPendingTask (this=<optimized out>, pending_task=...) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:342
#8  0x00007f3897bfd1aa in DoDelayedWork (next_delayed_work_time=0x7f388a930ad0, this=0x7f388a6facd8) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:469
#9  MessageLoop::DoDelayedWork (this=0x7f388a6facd8, next_delayed_work_time=0x7f388a930ad0) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:452
#10 0x00007f3897c0a771 in base::MessagePumpLibevent::Run (this=0x7f388a930ac0, delegate=0x7f388a6facd8) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_pump_libevent.cc:315
#11 0x00007f3897bfbbd6 in MessageLoop::RunInternal (this=0x7f388a6facd8) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:216
#12 0x00007f3897bfbbfe in RunHandler (this=0x7f388a6facd8) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:209
#13 MessageLoop::Run (this=0x7f388a6facd8) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/message_loop.cc:183
#14 0x00007f3897c014cb in base::Thread::ThreadMain (this=0x7f388a90e740) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/thread.cc:156
#15 0x00007f3897c0b118 in ThreadFunc (closure=<optimized out>) at /home/qdot/code/mozbuild/mozilla-central/ipc/chromium/src/base/platform_thread_posix.cc:39
#16 0x00007f389addbe9a in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#17 0x00007f389ab08cbd in clone () from /lib/x86_64-linux-gnu/libc.so.6
#18 0x0000000000000000 in ?? ()

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Check for shutdown in Connect/Accept in UnixSocket

Due to the fact that we can delay retries on accepts and connects, check in each of these functions to make sure we aren't trying to run after we've been asked to shut down.

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 730386 [details] [diff] [review]
Patch 1 (v1) - Check for shutdown in Connect/Accept in UnixSocket

Oops, wrong assignment. :)

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v2) - Moved connect delay to main thread

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 731332 [details] [diff] [review]
Patch 1 (v2) - Moved connect delay to main thread

Will fail if we call shutdown between the point connect() is called and the delayed task runs. Need to make task cancelable and a member.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v3) - Move connect delay to main thread cancelable task

Added ability to cancel and clear delayed connect tasks.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 731403 [details] [diff] [review]
Patch 1 (v3) - Move connect delay to main thread cancelable task

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

::: ipc/unixsocket/UnixSocket.cpp
@@ +127,5 @@
> +  void CancelDelayedConnectTask()
> +  {
> +    MOZ_ASSERT(NS_IsMainThread());
> +    if(!mDelayedConnectTask)
> +    {

Nits (here and below): space after if: "if (" and the opening brace goes on the same line as the if.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v4) - Move connect delay to main thread cancelable task

Nits picked

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2050e321ca34

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2050e321ca34