Skip to content

Commit

Permalink
Fix shutdown sequence to avoid SIGSEGV when running am command
Browse files Browse the repository at this point in the history
When the app_process is shutting down the main thread will close the
binder fd while pool threads are executing an ioctl (in
IPCThreadState::stopProcess called by AppRuntime::onStarted in
app_main.c).

The binder driver will then return all pending calls in ioctl
without any error and with a command. One of the threads gets a
BR_SPAWN_LOOPER which will create a new thread (the other thread
gets a BR_NOOP). This new thread then calls
vm->AttachCurrentThread. Usually this results in a log entry with
"AndroidRuntime: NOTE: attach of thread 'Binder Thread #3' failed",
but sometimes it also causes a SIGSEGV. This depends on the timing
between the new thread an the main thread that calls DestroyJavaVM
(in AndroidRuntime::start).

If IPCThreadState.cpp is compiled with "#define LOG_NDEBUG 0" the
pool thread will loop and hit the
ALOG_ASSERT(mProcess->mDriverFD >= 0) in
IPCThreadState::talkWithDriver.

Crashes like this has been seen when running the am command and
other commands that use the app_process.

This fix makes sure that any command that is received when the driver
fd is closed are ignored and IPCThreadState::talkWithDriver instead
returns an error which will cause the pool thread to exit and detach
itself from the vm. A check to avoid calling ioctl to a fd with -1
was also added in IPCThreadState::threadDestructor.

Another solution might be to change the binder driver so that it
returns an error when the fd is closed (or atleast not a
BR_SPAWN_LOOPER command). It might also be possible to call exit(0)
which is done when System.exit(0) is called from java.

Change-Id: I3d1f0ff64896c44be2a5994b3a90f7a06d27f429
  • Loading branch information
Johannes Carlsson authored and Jean-Baptiste Queru committed Jun 25, 2012
1 parent 589cdbe commit db1597a
Showing 1 changed file with 9 additions and 2 deletions.
11 changes: 9 additions & 2 deletions libs/binder/IPCThreadState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,9 @@ status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult)

status_t IPCThreadState::talkWithDriver(bool doReceive)
{
ALOG_ASSERT(mProcess->mDriverFD >= 0, "Binder driver is not opened");
if (mProcess->mDriverFD <= 0) {
return -EBADF;
}

binder_write_read bwr;

Expand Down Expand Up @@ -814,6 +816,9 @@ status_t IPCThreadState::talkWithDriver(bool doReceive)
#else
err = INVALID_OPERATION;
#endif
if (mProcess->mDriverFD <= 0) {
err = -EBADF;
}
IF_LOG_COMMANDS() {
alog << "Finished read/write, write size = " << mOut.dataSize() << endl;
}
Expand Down Expand Up @@ -1106,7 +1111,9 @@ void IPCThreadState::threadDestructor(void *st)
if (self) {
self->flushCommands();
#if defined(HAVE_ANDROID_OS)
ioctl(self->mProcess->mDriverFD, BINDER_THREAD_EXIT, 0);
if (self->mProcess->mDriverFD > 0) {
ioctl(self->mProcess->mDriverFD, BINDER_THREAD_EXIT, 0);
}
#endif
delete self;
}
Expand Down

0 comments on commit db1597a

Please sign in to comment.