From af567f73ac828b9c319c12fd92760c4c92f0dfa4 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Thu, 31 May 2012 16:15:35 -0700 Subject: [PATCH] Support looper callbacks based on smart pointers. Bug: 6559630 Change-Id: I5a667f219f431838638acefbc9fa6afa610971bd --- include/utils/Looper.h | 54 ++++++++++++++++++++++++++++++++++++++++-- libs/utils/Looper.cpp | 48 +++++++++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 14 deletions(-) diff --git a/include/utils/Looper.h b/include/utils/Looper.h index 84e3864a6..d4a0067fd 100644 --- a/include/utils/Looper.h +++ b/include/utils/Looper.h @@ -70,6 +70,9 @@ public: * A simple proxy that holds a weak reference to a message handler. */ class WeakMessageHandler : public MessageHandler { +protected: + virtual ~WeakMessageHandler(); + public: WeakMessageHandler(const wp& handler); virtual void handleMessage(const Message& message); @@ -79,6 +82,43 @@ private: }; +/** + * A looper callback. + */ +class LooperCallback : public virtual RefBase { +protected: + virtual ~LooperCallback() { } + +public: + /** + * Handles a poll event for the given file descriptor. + * It is given the file descriptor it is associated with, + * a bitmask of the poll events that were triggered (typically ALOOPER_EVENT_INPUT), + * and the data pointer that was originally supplied. + * + * Implementations should return 1 to continue receiving callbacks, or 0 + * to have this file descriptor and callback unregistered from the looper. + */ + virtual int handleEvent(int fd, int events, void* data) = 0; +}; + + +/** + * Wraps a ALooper_callbackFunc function pointer. + */ +class SimpleLooperCallback : public LooperCallback { +protected: + virtual ~SimpleLooperCallback(); + +public: + SimpleLooperCallback(ALooper_callbackFunc callback); + virtual int handleEvent(int fd, int events, void* data); + +private: + ALooper_callbackFunc mCallback; +}; + + /** * A polling loop that supports monitoring file descriptor events, optionally * using callbacks. The implementation uses epoll() internally. @@ -159,7 +199,7 @@ public: * If the same file descriptor was previously added, it is replaced. * * "fd" is the file descriptor to be added. - * "ident" is an identifier for this event, which is returned from ALooper_pollOnce(). + * "ident" is an identifier for this event, which is returned from pollOnce(). * The identifier must be >= 0, or ALOOPER_POLL_CALLBACK if providing a non-NULL callback. * "events" are the poll events to wake up on. Typically this is ALOOPER_EVENT_INPUT. * "callback" is the function to call when there is an event on the file descriptor. @@ -179,8 +219,14 @@ public: * * This method can be called on any thread. * This method may block briefly if it needs to wake the poll. + * + * The callback may either be specified as a bare function pointer or as a smart + * pointer callback object. The smart pointer should be preferred because it is + * easier to avoid races when the callback is removed from a different thread. + * See removeFd() for details. */ int addFd(int fd, int ident, int events, ALooper_callbackFunc callback, void* data); + int addFd(int fd, int ident, int events, const sp& callback, void* data); /** * Removes a previously added file descriptor from the looper. @@ -193,6 +239,10 @@ public: * by returning 0 or by calling this method, then it can be guaranteed to not be invoked * again at any later time unless registered anew. * + * A simple way to avoid this problem is to use the version of addFd() that takes + * a sp instead of a bare function pointer. The LooperCallback will + * be released at the appropriate time by the Looper. + * * Returns 1 if the file descriptor was removed, 0 if none was previously registered. * * This method can be called on any thread. @@ -273,7 +323,7 @@ private: struct Request { int fd; int ident; - ALooper_callbackFunc callback; + sp callback; void* data; }; diff --git a/libs/utils/Looper.cpp b/libs/utils/Looper.cpp index 989499362..a5e66458c 100644 --- a/libs/utils/Looper.cpp +++ b/libs/utils/Looper.cpp @@ -30,6 +30,9 @@ WeakMessageHandler::WeakMessageHandler(const wp& handler) : mHandler(handler) { } +WeakMessageHandler::~WeakMessageHandler() { +} + void WeakMessageHandler::handleMessage(const Message& message) { sp handler = mHandler.promote(); if (handler != NULL) { @@ -38,6 +41,20 @@ void WeakMessageHandler::handleMessage(const Message& message) { } +// --- SimpleLooperCallback --- + +SimpleLooperCallback::SimpleLooperCallback(ALooper_callbackFunc callback) : + mCallback(callback) { +} + +SimpleLooperCallback::~SimpleLooperCallback() { +} + +int SimpleLooperCallback::handleEvent(int fd, int events, void* data) { + return mCallback(fd, events, data); +} + + // --- Looper --- // Hint for number of file descriptors to be associated with the epoll instance. @@ -142,9 +159,8 @@ int Looper::pollOnce(int timeoutMillis, int* outFd, int* outEvents, void** outDa for (;;) { while (mResponseIndex < mResponses.size()) { const Response& response = mResponses.itemAt(mResponseIndex++); - ALooper_callbackFunc callback = response.request.callback; - if (!callback) { - int ident = response.request.ident; + int ident = response.request.ident; + if (ident >= 0) { int fd = response.request.fd; int events = response.events; void* data = response.request.data; @@ -165,7 +181,7 @@ int Looper::pollOnce(int timeoutMillis, int* outFd, int* outEvents, void** outDa ALOGD("%p ~ pollOnce - returning result %d", this, result); #endif if (outFd != NULL) *outFd = 0; - if (outEvents != NULL) *outEvents = NULL; + if (outEvents != NULL) *outEvents = 0; if (outData != NULL) *outData = NULL; return result; } @@ -293,20 +309,22 @@ Done: ; // Invoke all response callbacks. for (size_t i = 0; i < mResponses.size(); i++) { - const Response& response = mResponses.itemAt(i); - ALooper_callbackFunc callback = response.request.callback; - if (callback) { + Response& response = mResponses.editItemAt(i); + if (response.request.ident == ALOOPER_POLL_CALLBACK) { int fd = response.request.fd; int events = response.events; void* data = response.request.data; #if DEBUG_POLL_AND_WAKE || DEBUG_CALLBACKS ALOGD("%p ~ pollOnce - invoking fd event callback %p: fd=%d, events=0x%x, data=%p", - this, callback, fd, events, data); + this, response.request.callback.get(), fd, events, data); #endif - int callbackResult = callback(fd, events, data); + int callbackResult = response.request.callback->handleEvent(fd, events, data); if (callbackResult == 0) { removeFd(fd); } + // Clear the callback reference in the response structure promptly because we + // will not clear the response vector itself until the next poll. + response.request.callback.clear(); result = ALOOPER_POLL_CALLBACK; } } @@ -376,21 +394,27 @@ void Looper::pushResponse(int events, const Request& request) { } int Looper::addFd(int fd, int ident, int events, ALooper_callbackFunc callback, void* data) { + return addFd(fd, ident, events, callback ? new SimpleLooperCallback(callback) : NULL, data); +} + +int Looper::addFd(int fd, int ident, int events, const sp& callback, void* data) { #if DEBUG_CALLBACKS ALOGD("%p ~ addFd - fd=%d, ident=%d, events=0x%x, callback=%p, data=%p", this, fd, ident, - events, callback, data); + events, callback.get(), data); #endif - if (! callback) { + if (!callback.get()) { if (! mAllowNonCallbacks) { ALOGE("Invalid attempt to set NULL callback but not allowed for this looper."); return -1; } if (ident < 0) { - ALOGE("Invalid attempt to set NULL callback with ident <= 0."); + ALOGE("Invalid attempt to set NULL callback with ident < 0."); return -1; } + } else { + ident = ALOOPER_POLL_CALLBACK; } int epollEvents = 0;