emulator-fingerprint: Exit listener thread on HAL close
Note: This CL comes from Yu Ning <yu.ning@intel.com>
The current fingerprint HAL module for the emulator cannot be closed,
because fingerprint_close() always gets stuck waiting for the listener
thread to finish execution. Meanwhile, the listener thread is blocked on
qemud_channel_recv(), which does not return unless the host (QEMU) sends
something through the qemud channel.
This leads to the following bug:
https://code.google.com/p/android/issues/detail?id=186174
It is worth noting, though, that fingerprint_close() does try to unblock
qemud_channel_recv() by asynchronously closing the qemud channel, which
is backed by goldfish pipe. Unfortunately, that does not work, probably
because goldfish pipe does not automatically make pending I/O operations
return after closing a channel.
Any proper solution should probably poll the qemud channel before the
call to qemud_channel_recv(), and make sure the polling can be canceled,
allowing the listener thread to terminate gracefully.
Setting a timeout for poll() is straightforward to implement. Another
(slightly better) mechanism that enables instant interruption of the
polling of the qemud channel has also been implemented and tested, but
is more complicated and would add 200+ lines to fingerprint.c.
Therefore, take the simple, timeout-based approach for now, and maybe
revisit this issue in the future.
Change-Id: I02aa1f631d4ee651fe1ac7fb338076beaedddc40
diff --git a/fingerprint/fingerprint.c b/fingerprint/fingerprint.c
index 97b6d73..30ef4b7 100644
--- a/fingerprint/fingerprint.c
+++ b/fingerprint/fingerprint.c
@@ -26,6 +26,8 @@
#include <hardware/fingerprint.h>
#include <hardware/qemud.h>
+#include <poll.h>
+
#define FINGERPRINT_LISTEN_SERVICE_NAME "fingerprintlisten"
#define FINGERPRINT_FILENAME \
"/data/system/users/0/fpdata/emulator_fingerprint_storage.bin"
@@ -446,7 +448,7 @@
}
static worker_state_t getListenerState(qemu_fingerprint_device_t* dev) {
- ALOGD("----------------> %s ----------------->", __FUNCTION__);
+ ALOGV("----------------> %s ----------------->", __FUNCTION__);
worker_state_t state = STATE_IDLE;
pthread_mutex_lock(&dev->lock);
@@ -473,14 +475,59 @@
const char* cmd = "listen";
if (qemud_channel_send(qdev->qchanfd, cmd, strlen(cmd)) < 0) {
ALOGE("cannot write fingerprint 'listen' to host");
- return NULL;
+ goto done_quiet;
}
+
int comm_errors = 0;
- while (getListenerState(qdev) != STATE_EXIT) {
+ struct pollfd pfd = {
+ .fd = qdev->qchanfd,
+ .events = POLLIN,
+ };
+ while (1) {
int size = 0;
int fid = 0;
char buffer[MAX_COMM_CHARS] = {0};
- // will block until a new event happens
+ bool disconnected = false;
+ while (1) {
+ if (getListenerState(qdev) == STATE_EXIT) {
+ ALOGD("Received request to exit listener thread");
+ goto done;
+ }
+
+ // Reset revents before poll() (just to be safe)
+ pfd.revents = 0;
+
+ // Poll qemud channel for 5 seconds
+ // TODO: Eliminate the timeout so that polling can be interrupted
+ // instantly. One possible solution is to follow the example of
+ // android::Looper ($AOSP/system/core/include/utils/Looper.h and
+ // $AOSP/system/core/libutils/Looper.cpp), which makes use of an
+ // additional file descriptor ("wake event fd").
+ int nfds = poll(&pfd, 1, 5000);
+ if (nfds < 0) {
+ ALOGE("Could not poll qemud channel: %s", strerror(errno));
+ goto done;
+ }
+
+ if (!nfds) {
+ // poll() timed out - try again
+ continue;
+ }
+
+ // assert(nfds == 1)
+ if (pfd.revents & POLLIN) {
+ // Input data being available doesn't rule out a disconnection
+ disconnected = pfd.revents & (POLLERR | POLLHUP);
+ break; // Exit inner while loop
+ } else {
+ // Some event(s) other than "input data available" occurred,
+ // i.e. POLLERR or POLLHUP, indicating a disconnection
+ ALOGW("Lost connection to qemud channel");
+ goto done;
+ }
+ }
+
+ // Shouldn't block since we were just notified of a POLLIN event
if ((size = qemud_channel_recv(qdev->qchanfd, buffer,
sizeof(buffer) - 1)) > 0) {
buffer[size] = '\0';
@@ -511,6 +558,11 @@
} else {
ALOGE("Invalid command '%s' to fingerprint listener", buffer);
}
+
+ if (disconnected) {
+ ALOGW("Connection to qemud channel has been lost");
+ break;
+ }
} else {
ALOGE("fingerprint listener receive failure");
if (comm_errors > MAX_COMM_ERRORS)
@@ -518,7 +570,10 @@
}
}
+done:
ALOGD("Listener exit with %d receive errors", comm_errors);
+done_quiet:
+ close(qdev->qchanfd);
return NULL;
}
@@ -531,8 +586,7 @@
qemu_fingerprint_device_t* qdev = (qemu_fingerprint_device_t*)device;
pthread_mutex_lock(&qdev->lock);
- if (qdev->qchanfd != 0)
- close(qdev->qchanfd); // unblock listener
+ // Ask listener thread to exit
qdev->listener.state = STATE_EXIT;
pthread_mutex_unlock(&qdev->lock);
