From 90ed3e8d7883d9c80fb8bf11b1c593bd8b2b39d0 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Mon, 9 Sep 2013 23:36:25 -0700 Subject: [PATCH] fix a few problems with BitTube BitTube used to send objects one at a time and didn't handle errors properly. We now send all the objects in one call, which means they have to be read as a single batch as well. This changes the BitTube API. Update SensorService to the new API. Also added an API to set the size of the send buffer. Bug: 10641596 Change-Id: I77c70d35e351fdba0416fae4b7ca3b1d56272251 --- include/gui/BitTube.h | 31 ++++++++-- include/gui/SensorEventQueue.h | 6 ++ libs/gui/BitTube.cpp | 100 +++++++++++++++---------------- libs/gui/SensorEventQueue.cpp | 26 +++++--- services/sensorservice/SensorService.cpp | 8 ++- 5 files changed, 105 insertions(+), 66 deletions(-) diff --git a/include/gui/BitTube.h b/include/gui/BitTube.h index 3022d05f06..d32df846e8 100644 --- a/include/gui/BitTube.h +++ b/include/gui/BitTube.h @@ -33,30 +33,49 @@ class BitTube : public RefBase { public: - BitTube(); - BitTube(const Parcel& data); + // creates a BitTube with a default (4KB) send buffer + BitTube(); + + // creates a BitTube with a a specified send and receive buffer size + explicit BitTube(size_t bufsize); + + explicit BitTube(const Parcel& data); virtual ~BitTube(); + // check state after construction status_t initCheck() const; - int getFd() const; - ssize_t write(void const* vaddr, size_t size); - ssize_t read(void* vaddr, size_t size); - status_t writeToParcel(Parcel* reply) const; + // get receive file-descriptor + int getFd() const; + // send objects (sized blobs). All objects are guaranteed to be written or the call fails. template static ssize_t sendObjects(const sp& tube, T const* events, size_t count) { return sendObjects(tube, events, count, sizeof(T)); } + // receive objects (sized blobs). If the receiving buffer isn't large enough, + // excess messages are silently discarded. template static ssize_t recvObjects(const sp& tube, T* events, size_t count) { return recvObjects(tube, events, count, sizeof(T)); } + // parcels this BitTube + status_t writeToParcel(Parcel* reply) const; + private: + void init(size_t rcvbuf, size_t sndbuf); + + // send a message. The write is guaranteed to send the whole message or fail. + ssize_t write(void const* vaddr, size_t size); + + // receive a message. the passed buffer must be at least as large as the + // write call used to send the message, excess data is silently discarded. + ssize_t read(void* vaddr, size_t size); + int mSendFd; mutable int mReceiveFd; diff --git a/include/gui/SensorEventQueue.h b/include/gui/SensorEventQueue.h index 8d8493b482..a3a9daa6d4 100644 --- a/include/gui/SensorEventQueue.h +++ b/include/gui/SensorEventQueue.h @@ -49,6 +49,9 @@ class Looper; class SensorEventQueue : public ASensorEventQueue, public RefBase { public: + + enum { MAX_RECEIVE_BUFFER_EVENT_COUNT = 256 }; + SensorEventQueue(const sp& connection); virtual ~SensorEventQueue(); virtual void onFirstRef(); @@ -79,6 +82,9 @@ private: sp mSensorChannel; mutable Mutex mLock; mutable sp mLooper; + ASensorEvent* mRecBuffer; + size_t mAvailable; + size_t mConsumed; }; // ---------------------------------------------------------------------------- diff --git a/libs/gui/BitTube.cpp b/libs/gui/BitTube.cpp index cf44bb95fa..85a7de7e40 100644 --- a/libs/gui/BitTube.cpp +++ b/libs/gui/BitTube.cpp @@ -32,39 +32,26 @@ namespace android { // Socket buffer size. The default is typically about 128KB, which is much larger than // we really need. So we make it smaller. -static const size_t SOCKET_BUFFER_SIZE = 4 * 1024; +static const size_t DEFAULT_SOCKET_BUFFER_SIZE = 4 * 1024; BitTube::BitTube() : mSendFd(-1), mReceiveFd(-1) { - int sockets[2]; - if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets) == 0) { - int size = SOCKET_BUFFER_SIZE; - setsockopt(sockets[0], SOL_SOCKET, SO_SNDBUF, &size, sizeof(size)); - setsockopt(sockets[0], SOL_SOCKET, SO_RCVBUF, &size, sizeof(size)); - setsockopt(sockets[1], SOL_SOCKET, SO_SNDBUF, &size, sizeof(size)); - setsockopt(sockets[1], SOL_SOCKET, SO_RCVBUF, &size, sizeof(size)); - fcntl(sockets[0], F_SETFL, O_NONBLOCK); - fcntl(sockets[1], F_SETFL, O_NONBLOCK); - mReceiveFd = sockets[0]; - mSendFd = sockets[1]; - } else { - mReceiveFd = -errno; - ALOGE("BitTube: pipe creation failed (%s)", strerror(-mReceiveFd)); - } + init(DEFAULT_SOCKET_BUFFER_SIZE, DEFAULT_SOCKET_BUFFER_SIZE); +} + +BitTube::BitTube(size_t bufsize) + : mSendFd(-1), mReceiveFd(-1) +{ + init(bufsize, bufsize); } BitTube::BitTube(const Parcel& data) : mSendFd(-1), mReceiveFd(-1) { mReceiveFd = dup(data.readFileDescriptor()); - if (mReceiveFd >= 0) { - int size = SOCKET_BUFFER_SIZE; - setsockopt(mReceiveFd, SOL_SOCKET, SO_SNDBUF, &size, sizeof(size)); - setsockopt(mReceiveFd, SOL_SOCKET, SO_RCVBUF, &size, sizeof(size)); - fcntl(mReceiveFd, F_SETFL, O_NONBLOCK); - } else { + if (mReceiveFd < 0) { mReceiveFd = -errno; ALOGE("BitTube(Parcel): can't dup filedescriptor (%s)", strerror(-mReceiveFd)); @@ -80,6 +67,25 @@ BitTube::~BitTube() close(mReceiveFd); } +void BitTube::init(size_t rcvbuf, size_t sndbuf) { + int sockets[2]; + if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, sockets) == 0) { + size_t size = DEFAULT_SOCKET_BUFFER_SIZE; + setsockopt(sockets[0], SOL_SOCKET, SO_RCVBUF, &rcvbuf, sizeof(rcvbuf)); + setsockopt(sockets[1], SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf)); + // sine we don't use the "return channel", we keep it small... + setsockopt(sockets[0], SOL_SOCKET, SO_SNDBUF, &size, sizeof(size)); + setsockopt(sockets[1], SOL_SOCKET, SO_RCVBUF, &size, sizeof(size)); + fcntl(sockets[0], F_SETFL, O_NONBLOCK); + fcntl(sockets[1], F_SETFL, O_NONBLOCK); + mReceiveFd = sockets[0]; + mSendFd = sockets[1]; + } else { + mReceiveFd = -errno; + ALOGE("BitTube: pipe creation failed (%s)", strerror(-mReceiveFd)); + } +} + status_t BitTube::initCheck() const { if (mReceiveFd < 0) { @@ -98,10 +104,10 @@ ssize_t BitTube::write(void const* vaddr, size_t size) ssize_t err, len; do { len = ::send(mSendFd, vaddr, size, MSG_DONTWAIT | MSG_NOSIGNAL); + // cannot return less than size, since we're using SOCK_SEQPACKET err = len < 0 ? errno : 0; } while (err == EINTR); return err == 0 ? len : -err; - } ssize_t BitTube::read(void* vaddr, size_t size) @@ -134,39 +140,31 @@ status_t BitTube::writeToParcel(Parcel* reply) const ssize_t BitTube::sendObjects(const sp& tube, void const* events, size_t count, size_t objSize) { - ssize_t numObjects = 0; - for (size_t i=0 ; i(events) + objSize * i; - ssize_t size = tube->write(vaddr, objSize); - if (size < 0) { - // error occurred - return size; - } else if (size == 0) { - // no more space - break; - } - numObjects++; - } - return numObjects; + const char* vaddr = reinterpret_cast(events); + ssize_t size = tube->write(vaddr, count*objSize); + + // should never happen because of SOCK_SEQPACKET + LOG_ALWAYS_FATAL_IF((size >= 0) && (size % objSize), + "BitTube::sendObjects(count=%d, size=%d), res=%d (partial events were sent!)", + count, objSize, size); + + //ALOGE_IF(size<0, "error %d sending %d events", size, count); + return size < 0 ? size : size / objSize; } ssize_t BitTube::recvObjects(const sp& tube, void* events, size_t count, size_t objSize) { - ssize_t numObjects = 0; - for (size_t i=0 ; i(events) + objSize * i; - ssize_t size = tube->read(vaddr, objSize); - if (size < 0) { - // error occurred - return size; - } else if (size == 0) { - // no more messages - break; - } - numObjects++; - } - return numObjects; + char* vaddr = reinterpret_cast(events); + ssize_t size = tube->read(vaddr, count*objSize); + + // should never happen because of SOCK_SEQPACKET + LOG_ALWAYS_FATAL_IF((size >= 0) && (size % objSize), + "BitTube::recvObjects(count=%d, size=%d), res=%d (partial events were received!)", + count, objSize, size); + + //ALOGE_IF(size<0, "error %d receiving %d events", size, count); + return size < 0 ? size : size / objSize; } // ---------------------------------------------------------------------------- diff --git a/libs/gui/SensorEventQueue.cpp b/libs/gui/SensorEventQueue.cpp index 08b8ac89ec..ab50c1d730 100644 --- a/libs/gui/SensorEventQueue.cpp +++ b/libs/gui/SensorEventQueue.cpp @@ -35,12 +35,12 @@ namespace android { // ---------------------------------------------------------------------------- SensorEventQueue::SensorEventQueue(const sp& connection) - : mSensorEventConnection(connection) -{ + : mSensorEventConnection(connection), mRecBuffer(NULL), mAvailable(0), mConsumed(0) { + mRecBuffer = new ASensorEvent[MAX_RECEIVE_BUFFER_EVENT_COUNT]; } -SensorEventQueue::~SensorEventQueue() -{ +SensorEventQueue::~SensorEventQueue() { + delete [] mRecBuffer; } void SensorEventQueue::onFirstRef() @@ -59,9 +59,21 @@ ssize_t SensorEventQueue::write(const sp& tube, return BitTube::sendObjects(tube, events, numEvents); } -ssize_t SensorEventQueue::read(ASensorEvent* events, size_t numEvents) -{ - return BitTube::recvObjects(mSensorChannel, events, numEvents); +ssize_t SensorEventQueue::read(ASensorEvent* events, size_t numEvents) { + if (mAvailable == 0) { + ssize_t err = BitTube::recvObjects(mSensorChannel, + mRecBuffer, MAX_RECEIVE_BUFFER_EVENT_COUNT); + if (err < 0) { + return err; + } + mAvailable = err; + mConsumed = 0; + } + size_t count = numEvents < mAvailable ? numEvents : mAvailable; + memcpy(events, mRecBuffer + mConsumed, count*sizeof(ASensorEvent)); + mAvailable -= count; + mConsumed += count; + return count; } sp SensorEventQueue::getLooper() const diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 7bb6e86493..cb99531f93 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -292,8 +292,12 @@ bool SensorService::threadLoop() { ALOGD("nuSensorService thread starting..."); - const size_t numEventMax = 16; - const size_t minBufferSize = numEventMax + numEventMax * mVirtualSensorList.size(); + // each virtual sensor could generate an event per "real" event, that's why we need + // to size numEventMax much smaller than MAX_RECEIVE_BUFFER_EVENT_COUNT. + // in practice, this is too aggressive, but guaranteed to be enough. + const size_t minBufferSize = SensorEventQueue::MAX_RECEIVE_BUFFER_EVENT_COUNT; + const size_t numEventMax = minBufferSize / (1 + mVirtualSensorList.size()); + sensors_event_t buffer[minBufferSize]; sensors_event_t scratch[minBufferSize]; SensorDevice& device(SensorDevice::getInstance()); -- 2.11.0