diff --git a/include/utils/PollLoop.h b/include/utils/PollLoop.h index b3651caea..81230e84d 100644 --- a/include/utils/PollLoop.h +++ b/include/utils/PollLoop.h @@ -42,7 +42,7 @@ protected: virtual ~PollLoop(); public: - PollLoop(); + PollLoop(bool allowNonCallbacks); /** * A callback that it to be invoked when an event occurs on a file descriptor. @@ -54,6 +54,12 @@ public: */ typedef bool (*Callback)(int fd, int events, void* data); + enum { + POLL_CALLBACK = ALOOPER_POLL_CALLBACK, + POLL_TIMEOUT = ALOOPER_POLL_TIMEOUT, + POLL_ERROR = ALOOPER_POLL_ERROR, + }; + /** * Performs a single call to poll() with optional timeout in milliseconds. * Invokes callbacks for all file descriptors on which an event occurred. @@ -61,16 +67,25 @@ public: * If the timeout is zero, returns immediately without blocking. * If the timeout is negative, waits indefinitely until awoken. * - * Returns true if a callback was invoked or if the loop was awoken by wake(). - * Returns false if a timeout or error occurred. + * Returns ALOOPER_POLL_CALLBACK if a callback was invoked. * - * This method must only be called on the main thread. + * Returns ALOOPER_POLL_TIMEOUT if there was no data before the given + * timeout expired. + * + * Returns ALOPER_POLL_ERROR if an error occurred. + * + * Returns a value >= 0 containing a file descriptor if it has data + * and it has no callback function (requiring the caller here to handle it). + * In this (and only this) case outEvents and outData will contain the poll + * events and data associated with the fd. + * + * This method must only be called on the thread owning the PollLoop. * This method blocks until either a file descriptor is signalled, a timeout occurs, * or wake() is called. * This method does not return until it has finished invoking the appropriate callbacks * for all file descriptors that were signalled. */ - bool pollOnce(int timeoutMillis); + int32_t pollOnce(int timeoutMillis, int* outEvents = NULL, void** outData = NULL); /** * Wakes the loop asynchronously. @@ -80,6 +95,12 @@ public: */ void wake(); + /** + * Control whether this PollLoop instance allows using IDs instead + * of callbacks. + */ + bool getAllowNonCallbacks() const; + /** * Sets the callback for a file descriptor, replacing the existing one, if any. * It is an error to call this method with events == 0 or callback == NULL. @@ -95,7 +116,8 @@ public: /** * Like setCallback(), but for the NDK callback function. */ - void setLooperCallback(int fd, int events, ALooper_callbackFunc* callback, void* data); + void setLooperCallback(int fd, int events, ALooper_callbackFunc* callback, + void* data); /** * Removes the callback for a file descriptor, if one exists. @@ -141,7 +163,9 @@ private: ALooper_callbackFunc* looperCallback; void* data; }; - + + const bool mAllowNonCallbacks; + Mutex mLock; bool mPolling; uint32_t mWaiters; @@ -155,7 +179,9 @@ private: Vector mRequestedCallbacks; Vector mPendingCallbacks; // used privately by pollOnce - + Vector mPendingFds; // used privately by pollOnce + size_t mPendingFdsPos; + void openWakePipe(); void closeWakePipe(); diff --git a/libs/ui/InputDispatcher.cpp b/libs/ui/InputDispatcher.cpp index 42a7fc6e5..f809cba90 100644 --- a/libs/ui/InputDispatcher.cpp +++ b/libs/ui/InputDispatcher.cpp @@ -54,7 +54,7 @@ static inline nsecs_t now() { InputDispatcher::InputDispatcher(const sp& policy) : mPolicy(policy) { - mPollLoop = new PollLoop(); + mPollLoop = new PollLoop(false); mInboundQueue.head.refCount = -1; mInboundQueue.head.type = EventEntry::TYPE_SENTINEL; diff --git a/libs/utils/PollLoop.cpp b/libs/utils/PollLoop.cpp index 58fe1411d..f740fa0d5 100644 --- a/libs/utils/PollLoop.cpp +++ b/libs/utils/PollLoop.cpp @@ -25,8 +25,9 @@ static pthread_mutex_t gTLSMutex = PTHREAD_MUTEX_INITIALIZER; static bool gHaveTLS = false; static pthread_key_t gTLS = 0; -PollLoop::PollLoop() : - mPolling(false), mWaiters(0) { +PollLoop::PollLoop(bool allowNonCallbacks) : + mAllowNonCallbacks(allowNonCallbacks), mPolling(false), + mWaiters(0), mPendingFdsPos(0) { openWakePipe(); } @@ -106,7 +107,18 @@ void PollLoop::closeWakePipe() { // method is currently only called by the destructor. } -bool PollLoop::pollOnce(int timeoutMillis) { +int32_t PollLoop::pollOnce(int timeoutMillis, int* outEvents, void** outData) { + // If there are still pending fds from the last call, dispatch those + // first, to avoid an earlier fd from starving later ones. + const size_t pendingFdsCount = mPendingFds.size(); + if (mPendingFdsPos < pendingFdsCount) { + const PendingCallback& pending = mPendingFds.itemAt(mPendingFdsPos); + mPendingFdsPos++; + if (outEvents != NULL) *outEvents = pending.events; + if (outData != NULL) *outData = pending.data; + return pending.fd; + } + mLock.lock(); while (mWaiters != 0) { mResume.wait(mLock); @@ -114,7 +126,7 @@ bool PollLoop::pollOnce(int timeoutMillis) { mPolling = true; mLock.unlock(); - bool result; + int32_t result; size_t requestedCount = mRequestedFds.size(); #if DEBUG_POLL_AND_WAKE @@ -131,7 +143,7 @@ bool PollLoop::pollOnce(int timeoutMillis) { #if DEBUG_POLL_AND_WAKE LOGD("%p ~ pollOnce - timeout", this); #endif - result = false; + result = POLL_TIMEOUT; goto Done; } @@ -143,7 +155,7 @@ bool PollLoop::pollOnce(int timeoutMillis) { if (errno != EINTR) { LOGW("Poll failed with an unexpected error, errno=%d", errno); } - result = false; + result = POLL_ERROR; goto Done; } @@ -156,38 +168,44 @@ bool PollLoop::pollOnce(int timeoutMillis) { #endif mPendingCallbacks.clear(); + mPendingFds.clear(); + mPendingFdsPos = 0; + if (outEvents != NULL) *outEvents = 0; + if (outData != NULL) *outData = NULL; + + result = POLL_CALLBACK; for (size_t i = 0; i < requestedCount; i++) { const struct pollfd& requestedFd = mRequestedFds.itemAt(i); short revents = requestedFd.revents; if (revents) { const RequestedCallback& requestedCallback = mRequestedCallbacks.itemAt(i); - Callback callback = requestedCallback.callback; - ALooper_callbackFunc* looperCallback = requestedCallback.looperCallback; + PendingCallback pending; + pending.fd = requestedFd.fd; + pending.events = revents; + pending.callback = requestedCallback.callback; + pending.looperCallback = requestedCallback.looperCallback; + pending.data = requestedCallback.data; - if (callback || looperCallback) { - PendingCallback pendingCallback; - pendingCallback.fd = requestedFd.fd; - pendingCallback.events = requestedFd.revents; - pendingCallback.callback = callback; - pendingCallback.looperCallback = looperCallback; - pendingCallback.data = requestedCallback.data; - mPendingCallbacks.push(pendingCallback); - } else { - if (requestedFd.fd == mWakeReadPipeFd) { -#if DEBUG_POLL_AND_WAKE - LOGD("%p ~ pollOnce - awoken", this); -#endif - char buffer[16]; - ssize_t nRead; - do { - nRead = read(mWakeReadPipeFd, buffer, sizeof(buffer)); - } while (nRead == sizeof(buffer)); + if (pending.callback || pending.looperCallback) { + mPendingCallbacks.push(pending); + } else if (pending.fd != mWakeReadPipeFd) { + if (result == POLL_CALLBACK) { + result = pending.fd; + if (outEvents != NULL) *outEvents = pending.events; + if (outData != NULL) *outData = pending.data; } else { -#if DEBUG_POLL_AND_WAKE || DEBUG_CALLBACKS - LOGD("%p ~ pollOnce - fd %d has no callback!", this, requestedFd.fd); -#endif + mPendingFds.push(pending); } + } else { +#if DEBUG_POLL_AND_WAKE + LOGD("%p ~ pollOnce - awoken", this); +#endif + char buffer[16]; + ssize_t nRead; + do { + nRead = read(mWakeReadPipeFd, buffer, sizeof(buffer)); + } while (nRead == sizeof(buffer)); } respondedCount -= 1; @@ -196,7 +214,6 @@ bool PollLoop::pollOnce(int timeoutMillis) { } } } - result = true; Done: mLock.lock(); @@ -206,7 +223,7 @@ Done: } mLock.unlock(); - if (result) { + if (result == POLL_CALLBACK || result >= 0) { size_t pendingCount = mPendingCallbacks.size(); for (size_t i = 0; i < pendingCount; i++) { const PendingCallback& pendingCallback = mPendingCallbacks.itemAt(i); @@ -247,6 +264,10 @@ void PollLoop::wake() { } } +bool PollLoop::getAllowNonCallbacks() const { + return mAllowNonCallbacks; +} + void PollLoop::setCallback(int fd, int events, Callback callback, void* data) { setCallbackCommon(fd, events, callback, NULL, data); } @@ -263,12 +284,18 @@ void PollLoop::setCallbackCommon(int fd, int events, Callback callback, LOGD("%p ~ setCallback - fd=%d, events=%d", this, fd, events); #endif - if (! events || (! callback && ! looperCallback)) { - LOGE("Invalid attempt to set a callback with no selected poll events or no callback."); + if (! events) { + LOGE("Invalid attempt to set a callback with no selected poll events."); removeCallback(fd); return; } + if (! callback && ! looperCallback && ! mAllowNonCallbacks) { + LOGE("Invalid attempt to set NULL callback but not allowed."); + removeCallback(fd); + return; + } + wakeAndLock(); struct pollfd requestedFd; diff --git a/libs/utils/tests/PollLoop_test.cpp b/libs/utils/tests/PollLoop_test.cpp index 4848c0fa6..02f180893 100644 --- a/libs/utils/tests/PollLoop_test.cpp +++ b/libs/utils/tests/PollLoop_test.cpp @@ -87,7 +87,7 @@ protected: sp mPollLoop; virtual void SetUp() { - mPollLoop = new PollLoop(); + mPollLoop = new PollLoop(false); } virtual void TearDown() { @@ -98,26 +98,26 @@ protected: TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndNotAwoken_WaitsForTimeoutAndReturnsFalse) { StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(100); + int32_t result = mPollLoop->pollOnce(100); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(100, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal timeout"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; } TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndAwokenBeforeWaiting_ImmediatelyReturnsTrue) { mPollLoop->wake(); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(1000); + int32_t result = mPollLoop->pollOnce(1000); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. zero because wake() was called before waiting"; - EXPECT_TRUE(result) - << "pollOnce result should be true because loop was awoken"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because loop was awoken"; } TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndAwokenWhileWaiting_PromptlyReturnsTrue) { @@ -125,24 +125,24 @@ TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndAwokenWhileWaiting_PromptlyRe delayedWake->run(); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(1000); + int32_t result = mPollLoop->pollOnce(1000); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(100, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal wake delay"; - EXPECT_TRUE(result) - << "pollOnce result should be true because loop was awoken"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because loop was awoken"; } TEST_F(PollLoopTest, PollOnce_WhenZeroTimeoutAndNoRegisteredFDs_ImmediatelyReturnsFalse) { StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(0); + int32_t result = mPollLoop->pollOnce(0); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should be approx. zero"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; } TEST_F(PollLoopTest, PollOnce_WhenZeroTimeoutAndNoSignalledFDs_ImmediatelyReturnsFalse) { @@ -152,13 +152,13 @@ TEST_F(PollLoopTest, PollOnce_WhenZeroTimeoutAndNoSignalledFDs_ImmediatelyReturn handler.setCallback(mPollLoop, pipe.receiveFd, POLL_IN); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(0); + int32_t result = mPollLoop->pollOnce(0); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should be approx. zero"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; EXPECT_EQ(0, handler.callbackCount) << "callback should not have been invoked because FD was not signalled"; } @@ -171,13 +171,13 @@ TEST_F(PollLoopTest, PollOnce_WhenZeroTimeoutAndSignalledFD_ImmediatelyInvokesCa handler.setCallback(mPollLoop, pipe.receiveFd, POLL_IN); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(0); + int32_t result = mPollLoop->pollOnce(0); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should be approx. zero"; - EXPECT_TRUE(result) - << "pollOnce result should be true because FD was signalled"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because FD was signalled"; EXPECT_EQ(1, handler.callbackCount) << "callback should be invoked exactly once"; EXPECT_EQ(pipe.receiveFd, handler.fd) @@ -193,13 +193,13 @@ TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndNoSignalledFDs_WaitsForTimeou handler.setCallback(mPollLoop, pipe.receiveFd, POLL_IN); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(100); + int32_t result = mPollLoop->pollOnce(100); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); EXPECT_NEAR(100, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal timeout"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; EXPECT_EQ(0, handler.callbackCount) << "callback should not have been invoked because FD was not signalled"; } @@ -212,15 +212,15 @@ TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndSignalledFDBeforeWaiting_Imme handler.setCallback(mPollLoop, pipe.receiveFd, POLL_IN); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(100); + int32_t result = mPollLoop->pollOnce(100); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); ASSERT_EQ(OK, pipe.readSignal()) << "signal should actually have been written"; EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should be approx. zero"; - EXPECT_TRUE(result) - << "pollOnce result should be true because FD was signalled"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because FD was signalled"; EXPECT_EQ(1, handler.callbackCount) << "callback should be invoked exactly once"; EXPECT_EQ(pipe.receiveFd, handler.fd) @@ -238,15 +238,15 @@ TEST_F(PollLoopTest, PollOnce_WhenNonZeroTimeoutAndSignalledFDWhileWaiting_Promp delayedWriteSignal->run(); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(1000); + int32_t result = mPollLoop->pollOnce(1000); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); ASSERT_EQ(OK, pipe.readSignal()) << "signal should actually have been written"; EXPECT_NEAR(100, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal signal delay"; - EXPECT_TRUE(result) - << "pollOnce result should be true because FD was signalled"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because FD was signalled"; EXPECT_EQ(1, handler.callbackCount) << "callback should be invoked exactly once"; EXPECT_EQ(pipe.receiveFd, handler.fd) @@ -264,15 +264,15 @@ TEST_F(PollLoopTest, PollOnce_WhenCallbackAddedThenRemoved_CallbackShouldNotBeIn mPollLoop->removeCallback(pipe.receiveFd); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(100); + int32_t result = mPollLoop->pollOnce(100); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); ASSERT_EQ(OK, pipe.readSignal()) << "signal should actually have been written"; EXPECT_NEAR(100, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal timeout because FD was no longer registered"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; EXPECT_EQ(0, handler.callbackCount) << "callback should not be invoked"; } @@ -287,15 +287,15 @@ TEST_F(PollLoopTest, PollOnce_WhenCallbackReturnsFalse_CallbackShouldNotBeInvoke pipe.writeSignal(); StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(0); + int32_t result = mPollLoop->pollOnce(0); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); ASSERT_EQ(OK, pipe.readSignal()) << "signal should actually have been written"; EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal zero because FD was already signalled"; - EXPECT_TRUE(result) - << "pollOnce result should be true because FD was signalled"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because FD was signalled"; EXPECT_EQ(1, handler.callbackCount) << "callback should be invoked"; @@ -310,8 +310,8 @@ TEST_F(PollLoopTest, PollOnce_WhenCallbackReturnsFalse_CallbackShouldNotBeInvoke << "signal should actually have been written"; EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. equal zero because timeout was zero"; - EXPECT_FALSE(result) - << "pollOnce result should be false because timeout occurred"; + EXPECT_EQ(result, PollLoop::POLL_TIMEOUT) + << "pollOnce result should be POLL_TIMEOUT"; EXPECT_EQ(1, handler.callbackCount) << "callback should not be invoked this time"; } @@ -351,15 +351,15 @@ TEST_F(PollLoopTest, PollOnce_WhenCallbackAddedTwice_OnlySecondCallbackShouldBeI pipe.writeSignal(); // would cause FD to be considered signalled StopWatch stopWatch("pollOnce"); - bool result = mPollLoop->pollOnce(100); + int32_t result = mPollLoop->pollOnce(100); int32_t elapsedMillis = ns2ms(stopWatch.elapsedTime()); ASSERT_EQ(OK, pipe.readSignal()) << "signal should actually have been written"; EXPECT_NEAR(0, elapsedMillis, TIMING_TOLERANCE_MS) << "elapsed time should approx. zero because FD was already signalled"; - EXPECT_TRUE(result) - << "pollOnce result should be true because FD was signalled"; + EXPECT_EQ(result, PollLoop::POLL_CALLBACK) + << "pollOnce result should be POLL_CALLBACK because FD was signalled"; EXPECT_EQ(0, handler1.callbackCount) << "original handler callback should not be invoked because it was replaced"; EXPECT_EQ(1, handler2.callbackCount)