am 93176114: am a7eb4b74: Merge "Fix sockfd leakage in SensorService." into lmp-dev
* commit '93176114cf34c9874c2db70fd7665e9f95dda9d0':
Fix sockfd leakage in SensorService.
diff --git a/include/gui/SensorEventQueue.h b/include/gui/SensorEventQueue.h
index 019308d..02b3d38 100644
--- a/include/gui/SensorEventQueue.h
+++ b/include/gui/SensorEventQueue.h
@@ -86,6 +86,7 @@
ASensorEvent* mRecBuffer;
size_t mAvailable;
size_t mConsumed;
+ uint32_t mNumAcksToSend;
};
// ----------------------------------------------------------------------------
diff --git a/libs/gui/SensorEventQueue.cpp b/libs/gui/SensorEventQueue.cpp
index 842502d..1305e9f 100644
--- a/libs/gui/SensorEventQueue.cpp
+++ b/libs/gui/SensorEventQueue.cpp
@@ -36,7 +36,8 @@
// ----------------------------------------------------------------------------
SensorEventQueue::SensorEventQueue(const sp<ISensorEventConnection>& connection)
- : mSensorEventConnection(connection), mRecBuffer(NULL), mAvailable(0), mConsumed(0) {
+ : mSensorEventConnection(connection), mRecBuffer(NULL), mAvailable(0), mConsumed(0),
+ mNumAcksToSend(0) {
mRecBuffer = new ASensorEvent[MAX_RECEIVE_BUFFER_EVENT_COUNT];
}
@@ -148,14 +149,17 @@
void SensorEventQueue::sendAck(const ASensorEvent* events, int count) {
for (int i = 0; i < count; ++i) {
if (events[i].flags & WAKE_UP_SENSOR_EVENT_NEEDS_ACK) {
- // Send just a byte of data to acknowledge for the wake up sensor events
- // received
- char buf = '1';
- ssize_t size = ::send(mSensorChannel->getFd(), &buf, sizeof(buf),
- MSG_DONTWAIT | MSG_NOSIGNAL);
- if (size < 0) {
- ALOGE("sendAck failure %d", size);
- }
+ ++mNumAcksToSend;
+ }
+ }
+ // Send mNumAcksToSend to acknowledge for the wake up sensor events received.
+ if (mNumAcksToSend > 0) {
+ ssize_t size = ::send(mSensorChannel->getFd(), &mNumAcksToSend, sizeof(mNumAcksToSend),
+ MSG_DONTWAIT | MSG_NOSIGNAL);
+ if (size < 0) {
+ ALOGE("sendAck failure %d %d", size, mNumAcksToSend);
+ } else {
+ mNumAcksToSend = 0;
}
}
return;
diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp
index 7c57957..8831893 100644
--- a/services/sensorservice/SensorService.cpp
+++ b/services/sensorservice/SensorService.cpp
@@ -66,7 +66,8 @@
const char* SensorService::WAKE_LOCK_NAME = "SensorService";
SensorService::SensorService()
- : mInitCheck(NO_INIT)
+ : mInitCheck(NO_INIT), mSocketBufferSize(SOCKET_BUFFER_SIZE_NON_BATCHED),
+ mWakeLockAcquired(false)
{
}
@@ -433,7 +434,6 @@
if (bufferHasWakeUpEvent && !mWakeLockAcquired) {
acquire_wake_lock(PARTIAL_WAKE_LOCK, WAKE_LOCK_NAME);
mWakeLockAcquired = true;
- ALOGD_IF(DEBUG_CONNECTIONS, "acquired wakelock %s", WAKE_LOCK_NAME);
}
recordLastValueLocked(mSensorEventBuffer, count);
@@ -523,7 +523,6 @@
if (mWakeLockAcquired && !needsWakeLock) {
release_wake_lock(WAKE_LOCK_NAME);
mWakeLockAcquired = false;
- ALOGD_IF(DEBUG_CONNECTIONS, "released wakelock %s", WAKE_LOCK_NAME);
}
} while (!Thread::exitPending());
@@ -650,6 +649,7 @@
if (sensor) {
sensor->activate(c, false);
}
+ c->removeSensor(handle);
}
SensorRecord* rec = mActiveSensors.valueAt(i);
ALOGE_IF(!rec, "mActiveSensors[%zu] is null (handle=0x%08x)!", i, handle);
@@ -667,6 +667,7 @@
i++;
}
}
+ c->updateLooperRegistration(mLooper);
mActiveConnections.remove(connection);
BatteryService::cleanup(c->getUid());
if (c->needsWakeLock()) {
@@ -770,10 +771,8 @@
err = sensor->activate(connection.get(), true);
}
- if (err == NO_ERROR && sensor->getSensor().isWakeUpSensor()) {
- // Add the file descriptor to the Looper for receiving acknowledgments;
- int ret = mLooper->addFd(connection->getSensorChannel()->getSendFd(), 0,
- ALOOPER_EVENT_INPUT, connection.get(), NULL);
+ if (err == NO_ERROR) {
+ connection->updateLooperRegistration(mLooper);
}
if (err != NO_ERROR) {
@@ -813,6 +812,7 @@
BatteryService::disableSensor(connection->getUid(), handle);
}
if (connection->hasAnySensor() == false) {
+ connection->updateLooperRegistration(mLooper);
mActiveConnections.remove(connection);
}
// see if this sensor becomes inactive
@@ -866,11 +866,10 @@
err = INVALID_OPERATION;
continue;
}
- SensorEventConnection::FlushInfo& flushInfo = connection->mSensorInfo.editValueFor(handle);
if (halVersion <= SENSORS_DEVICE_API_VERSION_1_0 || isVirtualSensor(handle)) {
// For older devices just increment pending flush count which will send a trivial
// flush complete event.
- flushInfo.mPendingFlushEventsToSend++;
+ connection->incrementPendingFlushCount(handle);
} else {
status_t err_flush = sensor->flush(connection.get(), handle);
if (err_flush == NO_ERROR) {
@@ -922,7 +921,6 @@
}
}
if (releaseLock) {
- ALOGD_IF(DEBUG_CONNECTIONS, "releasing wakelock %s", WAKE_LOCK_NAME);
release_wake_lock(WAKE_LOCK_NAME);
mWakeLockAcquired = false;
}
@@ -955,6 +953,7 @@
// Remove this connections from the queue of flush() calls made on this sensor.
for (Vector< wp<SensorEventConnection> >::iterator it =
mPendingFlushConnections.begin(); it != mPendingFlushConnections.end();) {
+
if (it->unsafe_get() == connection.unsafe_get()) {
it = mPendingFlushConnections.erase(it);
} else {
@@ -987,9 +986,8 @@
SensorService::SensorEventConnection::SensorEventConnection(
const sp<SensorService>& service, uid_t uid)
- : mService(service), mUid(uid), mWakeLockRefCount(0), mEventCache(NULL), mCacheSize(0),
- mMaxCacheSize(0) {
- const SensorDevice& device(SensorDevice::getInstance());
+ : mService(service), mUid(uid), mWakeLockRefCount(0), mHasLooperCallbacks(false),
+ mDead(false), mEventCache(NULL), mCacheSize(0), mMaxCacheSize(0) {
mChannel = new BitTube(mService->mSocketBufferSize);
#if DEBUG_CONNECTIONS
mEventsReceived = mEventsSentFromCache = mEventsSent = 0;
@@ -1011,7 +1009,7 @@
bool SensorService::SensorEventConnection::needsWakeLock() {
Mutex::Autolock _l(mConnectionLock);
- return mWakeLockRefCount > 0;
+ return !mDead && mWakeLockRefCount > 0;
}
void SensorService::SensorEventConnection::dump(String8& result) {
@@ -1090,6 +1088,65 @@
}
}
+void SensorService::SensorEventConnection::updateLooperRegistration(const sp<Looper>& looper) {
+ Mutex::Autolock _l(mConnectionLock);
+ updateLooperRegistrationLocked(looper);
+}
+
+void SensorService::SensorEventConnection::updateLooperRegistrationLocked(
+ const sp<Looper>& looper) {
+ bool isConnectionActive = mSensorInfo.size() > 0;
+ // If all sensors are unregistered OR Looper has encountered an error, we
+ // can remove the Fd from the Looper if it has been previously added.
+ if (!isConnectionActive || mDead) {
+ if (mHasLooperCallbacks) {
+ ALOGD_IF(DEBUG_CONNECTIONS, "%p removeFd fd=%d", this, mChannel->getSendFd());
+ looper->removeFd(mChannel->getSendFd());
+ mHasLooperCallbacks = false;
+ }
+ return;
+ }
+
+ int looper_flags = 0;
+ if (mCacheSize > 0) looper_flags |= ALOOPER_EVENT_OUTPUT;
+ for (size_t i = 0; i < mSensorInfo.size(); ++i) {
+ const int handle = mSensorInfo.keyAt(i);
+ if (mService->getSensorFromHandle(handle).isWakeUpSensor()) {
+ looper_flags |= ALOOPER_EVENT_INPUT;
+ break;
+ }
+ }
+ // If flags is still set to zero, we don't need to add this fd to the Looper, if
+ // the fd has already been added, remove it. This is likely to happen when ALL the
+ // events stored in the cache have been sent to the corresponding app.
+ if (looper_flags == 0) {
+ if (mHasLooperCallbacks) {
+ ALOGD_IF(DEBUG_CONNECTIONS, "removeFd fd=%d", mChannel->getSendFd());
+ looper->removeFd(mChannel->getSendFd());
+ mHasLooperCallbacks = false;
+ }
+ return;
+ }
+ // Add the file descriptor to the Looper for receiving acknowledegments if the app has
+ // registered for wake-up sensors OR for sending events in the cache.
+ int ret = looper->addFd(mChannel->getSendFd(), 0, looper_flags, this, NULL);
+ if (ret == 1) {
+ ALOGD_IF(DEBUG_CONNECTIONS, "%p addFd fd=%d", this, mChannel->getSendFd());
+ mHasLooperCallbacks = true;
+ } else {
+ ALOGE("Looper::addFd failed ret=%d fd=%d", ret, mChannel->getSendFd());
+ }
+}
+
+void SensorService::SensorEventConnection::incrementPendingFlushCount(int32_t handle) {
+ Mutex::Autolock _l(mConnectionLock);
+ ssize_t index = mSensorInfo.indexOfKey(handle);
+ if (index >= 0) {
+ FlushInfo& flushInfo = mSensorInfo.editValueAt(index);
+ flushInfo.mPendingFlushEventsToSend++;
+ }
+}
+
status_t SensorService::SensorEventConnection::sendEvents(
sensors_event_t const* buffer, size_t numEvents,
sensors_event_t* scratch,
@@ -1104,8 +1161,9 @@
if (buffer[i].type == SENSOR_TYPE_META_DATA) {
ALOGD_IF(DEBUG_CONNECTIONS, "flush complete event sensor==%d ",
buffer[i].meta_data.sensor);
- // Setting sensor_handle to the correct sensor to ensure the sensor events per connection are
- // filtered correctly. buffer[i].sensor is zero for meta_data events.
+ // Setting sensor_handle to the correct sensor to ensure the sensor events per
+ // connection are filtered correctly. buffer[i].sensor is zero for meta_data
+ // events.
sensor_handle = buffer[i].meta_data.sensor;
}
ssize_t index = mSensorInfo.indexOfKey(sensor_handle);
@@ -1233,8 +1291,7 @@
// Add this file descriptor to the looper to get a callback when this fd is available for
// writing.
- mService->getLooper()->addFd(mChannel->getSendFd(), 0,
- ALOOPER_EVENT_OUTPUT | ALOOPER_EVENT_INPUT, this, NULL);
+ updateLooperRegistrationLocked(mService->getLooper());
return size;
}
@@ -1340,8 +1397,9 @@
ALOGD_IF(DEBUG_CONNECTIONS, "wrote all events from cache size=%d ", mCacheSize);
// All events from the cache have been sent. Reset cache size to zero.
mCacheSize = 0;
- // Poll only for ALOOPER_EVENT_INPUT(read) on the file descriptor.
- mService->getLooper()->addFd(mChannel->getSendFd(), 0, ALOOPER_EVENT_INPUT, this, NULL);
+ // There are no more events in the cache. We don't need to poll for write on the fd.
+ // Update Looper registration.
+ updateLooperRegistrationLocked(mService->getLooper());
}
void SensorService::SensorEventConnection::countFlushCompleteEventsLocked(
@@ -1400,22 +1458,38 @@
return mService->flushSensor(this);
}
-int SensorService::SensorEventConnection::handleEvent(int fd, int events, void* data) {
+int SensorService::SensorEventConnection::handleEvent(int fd, int events, void* /*data*/) {
if (events & ALOOPER_EVENT_HANGUP || events & ALOOPER_EVENT_ERROR) {
- return 0;
+ {
+ // If the Looper encounters some error, set the flag mDead, reset mWakeLockRefCount,
+ // and remove the fd from Looper. Call checkWakeLockState to know if SensorService
+ // can release the wake-lock.
+ ALOGD_IF(DEBUG_CONNECTIONS, "%p Looper error %d", this, fd);
+ Mutex::Autolock _l(mConnectionLock);
+ mDead = true;
+ mWakeLockRefCount = 0;
+ updateLooperRegistrationLocked(mService->getLooper());
+ }
+ mService->checkWakeLockState();
+ return 1;
}
if (events & ALOOPER_EVENT_INPUT) {
- char buf;
- ssize_t ret = ::recv(fd, &buf, sizeof(buf), MSG_DONTWAIT);
-
+ uint32_t numAcks = 0;
+ ssize_t ret = ::recv(fd, &numAcks, sizeof(numAcks), MSG_DONTWAIT);
{
Mutex::Autolock _l(mConnectionLock);
- if (mWakeLockRefCount > 0) {
- --mWakeLockRefCount;
+ // Sanity check to ensure there are no read errors in recv, numAcks is always
+ // within the range and not zero. If any of the above don't hold reset mWakeLockRefCount
+ // to zero.
+ if (ret != sizeof(numAcks) || numAcks > mWakeLockRefCount || numAcks == 0) {
+ ALOGE("Looper read error ret=%d numAcks=%d", ret, numAcks);
+ mWakeLockRefCount = 0;
+ } else {
+ mWakeLockRefCount -= numAcks;
}
#if DEBUG_CONNECTIONS
- ++mTotalAcksReceived;
+ mTotalAcksReceived += numAcks;
#endif
}
// Check if wakelock can be released by sensorservice. mConnectionLock needs to be released
diff --git a/services/sensorservice/SensorService.h b/services/sensorservice/SensorService.h
index 0fcafac..5e3eeb5 100644
--- a/services/sensorservice/SensorService.h
+++ b/services/sensorservice/SensorService.h
@@ -117,6 +117,20 @@
// If this fd is available for writing send the data from the cache.
virtual int handleEvent(int fd, int events, void* data);
+ // Increment mPendingFlushEventsToSend for the given sensor handle.
+ void incrementPendingFlushCount(int32_t handle);
+
+ // Add or remove the file descriptor associated with the BitTube to the looper. If mDead is
+ // set to true or there are no more sensors for this connection, the file descriptor is
+ // removed if it has been previously added to the Looper. Depending on the state of the
+ // connection FD may be added to the Looper. The flags to set are determined by the internal
+ // state of the connection. FDs are added to the looper when wake-up sensors are registered
+ // (to poll for acknowledgements) and when write fails on the socket when there are too many
+ // events (to poll when the FD is available for writing). FDs are removed when there is an
+ // error and the other end hangs up or when this client unregisters for this connection.
+ void updateLooperRegistration(const sp<Looper>& looper);
+ void updateLooperRegistrationLocked(const sp<Looper>& looper);
+
sp<SensorService> const mService;
sp<BitTube> mChannel;
uid_t mUid;
@@ -124,8 +138,17 @@
// Number of events from wake up sensors which are still pending and haven't been delivered
// to the corresponding application. It is incremented by one unit for each write to the
// socket.
- int mWakeLockRefCount;
+ uint32_t mWakeLockRefCount;
+ // If this flag is set to true, it means that the file descriptor associated with the
+ // BitTube has been added to the Looper in SensorService. This flag is typically set when
+ // this connection has wake-up sensors associated with it or when write has failed on this
+ // connection and we're storing some events in the cache.
+ bool mHasLooperCallbacks;
+ // If there are any errors associated with the Looper this flag is set to true and
+ // mWakeLockRefCount is reset to zero. needsWakeLock method will always return false, if
+ // this flag is set.
+ bool mDead;
struct FlushInfo {
// The number of flush complete events dropped for this sensor is stored here.
// They are sent separately before the next batch of events.
@@ -222,7 +245,7 @@
status_t mInitCheck;
// Socket buffersize used to initialize BitTube. This size depends on whether batching is
// supported or not.
- size_t mSocketBufferSize;
+ uint32_t mSocketBufferSize;
sp<Looper> mLooper;
// protected by mLock