From 80a1de1007ddc62e1af2a4746008f499145aeaab Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Thu, 31 May 2012 16:23:11 -0700 Subject: Use sp to fix race causing dangling pointer. Bug: 6559630 Change-Id: I9b9c76577779841006f9c024a80685ba8b7cd0e1 --- core/jni/android_view_DisplayEventReceiver.cpp | 42 ++++++++++++++------------ core/jni/android_view_InputEventReceiver.cpp | 35 ++++++++++----------- 2 files changed, 40 insertions(+), 37 deletions(-) diff --git a/core/jni/android_view_DisplayEventReceiver.cpp b/core/jni/android_view_DisplayEventReceiver.cpp index d80bfb3..89058a7 100644 --- a/core/jni/android_view_DisplayEventReceiver.cpp +++ b/core/jni/android_view_DisplayEventReceiver.cpp @@ -42,12 +42,13 @@ static struct { } gDisplayEventReceiverClassInfo; -class NativeDisplayEventReceiver : public RefBase { +class NativeDisplayEventReceiver : public LooperCallback { public: NativeDisplayEventReceiver(JNIEnv* env, jobject receiverObj, const sp& messageQueue); status_t initialize(); + void dispose(); status_t scheduleVsync(); protected: @@ -59,7 +60,7 @@ private: DisplayEventReceiver mReceiver; bool mWaitingForVsync; - static int handleReceiveCallback(int receiveFd, int events, void* data); + virtual int handleEvent(int receiveFd, int events, void* data); bool readLastVsyncMessage(nsecs_t* outTimestamp, uint32_t* outCount); }; @@ -72,12 +73,6 @@ NativeDisplayEventReceiver::NativeDisplayEventReceiver(JNIEnv* env, } NativeDisplayEventReceiver::~NativeDisplayEventReceiver() { - ALOGV("receiver %p ~ Disposing display event receiver.", this); - - if (!mReceiver.initCheck()) { - mMessageQueue->getLooper()->removeFd(mReceiver.getFd()); - } - JNIEnv* env = AndroidRuntime::getJNIEnv(); env->DeleteGlobalRef(mReceiverObjGlobal); } @@ -90,13 +85,21 @@ status_t NativeDisplayEventReceiver::initialize() { } int rc = mMessageQueue->getLooper()->addFd(mReceiver.getFd(), 0, ALOOPER_EVENT_INPUT, - handleReceiveCallback, this); + this, NULL); if (rc < 0) { return UNKNOWN_ERROR; } return OK; } +void NativeDisplayEventReceiver::dispose() { + ALOGV("receiver %p ~ Disposing display event receiver.", this); + + if (!mReceiver.initCheck()) { + mMessageQueue->getLooper()->removeFd(mReceiver.getFd()); + } +} + status_t NativeDisplayEventReceiver::scheduleVsync() { if (!mWaitingForVsync) { ALOGV("receiver %p ~ Scheduling vsync.", this); @@ -117,9 +120,7 @@ status_t NativeDisplayEventReceiver::scheduleVsync() { return OK; } -int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events, void* data) { - sp r = static_cast(data); - +int NativeDisplayEventReceiver::handleEvent(int receiveFd, int events, void* data) { if (events & (ALOOPER_EVENT_ERROR | ALOOPER_EVENT_HANGUP)) { ALOGE("Display event receiver pipe was closed or an error occurred. " "events=0x%x", events); @@ -135,23 +136,23 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events, // Drain all pending events, keep the last vsync. nsecs_t vsyncTimestamp; uint32_t vsyncCount; - if (!r->readLastVsyncMessage(&vsyncTimestamp, &vsyncCount)) { - ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", data); + if (!readLastVsyncMessage(&vsyncTimestamp, &vsyncCount)) { + ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", this); return 1; // keep the callback, did not obtain a vsync pulse } ALOGV("receiver %p ~ Vsync pulse: timestamp=%lld, count=%d", - data, vsyncTimestamp, vsyncCount); - r->mWaitingForVsync = false; + this, vsyncTimestamp, vsyncCount); + mWaitingForVsync = false; JNIEnv* env = AndroidRuntime::getJNIEnv(); - ALOGV("receiver %p ~ Invoking vsync handler.", data); - env->CallVoidMethod(r->mReceiverObjGlobal, + ALOGV("receiver %p ~ Invoking vsync handler.", this); + env->CallVoidMethod(mReceiverObjGlobal, gDisplayEventReceiverClassInfo.dispatchVsync, vsyncTimestamp, vsyncCount); - ALOGV("receiver %p ~ Returned from vsync handler.", data); + ALOGV("receiver %p ~ Returned from vsync handler.", this); - r->mMessageQueue->raiseAndClearException(env, "dispatchVsync"); + mMessageQueue->raiseAndClearException(env, "dispatchVsync"); return 1; // keep the callback } @@ -201,6 +202,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj, static void nativeDispose(JNIEnv* env, jclass clazz, jint receiverPtr) { sp receiver = reinterpret_cast(receiverPtr); + receiver->dispose(); receiver->decStrong(gDisplayEventReceiverClassInfo.clazz); // drop reference held by the object } diff --git a/core/jni/android_view_InputEventReceiver.cpp b/core/jni/android_view_InputEventReceiver.cpp index 08e08b9..9501cf2 100644 --- a/core/jni/android_view_InputEventReceiver.cpp +++ b/core/jni/android_view_InputEventReceiver.cpp @@ -44,13 +44,14 @@ static struct { } gInputEventReceiverClassInfo; -class NativeInputEventReceiver : public RefBase { +class NativeInputEventReceiver : public LooperCallback { public: NativeInputEventReceiver(JNIEnv* env, jobject receiverObj, const sp& inputChannel, const sp& messageQueue); status_t initialize(); + void dispose(); status_t finishInputEvent(uint32_t seq, bool handled); status_t consumeEvents(JNIEnv* env, bool consumeBatches, nsecs_t frameTime); @@ -68,7 +69,7 @@ private: return mInputConsumer.getChannel()->getName().string(); } - static int handleReceiveCallback(int receiveFd, int events, void* data); + virtual int handleEvent(int receiveFd, int events, void* data); }; @@ -84,23 +85,24 @@ NativeInputEventReceiver::NativeInputEventReceiver(JNIEnv* env, } NativeInputEventReceiver::~NativeInputEventReceiver() { -#if DEBUG_DISPATCH_CYCLE - ALOGD("channel '%s' ~ Disposing input event receiver.", getInputChannelName()); -#endif - - mMessageQueue->getLooper()->removeFd(mInputConsumer.getChannel()->getFd()); - JNIEnv* env = AndroidRuntime::getJNIEnv(); env->DeleteGlobalRef(mReceiverObjGlobal); } status_t NativeInputEventReceiver::initialize() { int receiveFd = mInputConsumer.getChannel()->getFd(); - mMessageQueue->getLooper()->addFd( - receiveFd, 0, ALOOPER_EVENT_INPUT, handleReceiveCallback, this); + mMessageQueue->getLooper()->addFd(receiveFd, 0, ALOOPER_EVENT_INPUT, this, NULL); return OK; } +void NativeInputEventReceiver::dispose() { +#if DEBUG_DISPATCH_CYCLE + ALOGD("channel '%s' ~ Disposing input event receiver.", getInputChannelName()); +#endif + + mMessageQueue->getLooper()->removeFd(mInputConsumer.getChannel()->getFd()); +} + status_t NativeInputEventReceiver::finishInputEvent(uint32_t seq, bool handled) { #if DEBUG_DISPATCH_CYCLE ALOGD("channel '%s' ~ Finished input event.", getInputChannelName()); @@ -114,24 +116,22 @@ status_t NativeInputEventReceiver::finishInputEvent(uint32_t seq, bool handled) return status; } -int NativeInputEventReceiver::handleReceiveCallback(int receiveFd, int events, void* data) { - sp r = static_cast(data); - +int NativeInputEventReceiver::handleEvent(int receiveFd, int events, void* data) { if (events & (ALOOPER_EVENT_ERROR | ALOOPER_EVENT_HANGUP)) { ALOGE("channel '%s' ~ Publisher closed input channel or an error occurred. " - "events=0x%x", r->getInputChannelName(), events); + "events=0x%x", getInputChannelName(), events); return 0; // remove the callback } if (!(events & ALOOPER_EVENT_INPUT)) { ALOGW("channel '%s' ~ Received spurious callback for unhandled poll event. " - "events=0x%x", r->getInputChannelName(), events); + "events=0x%x", getInputChannelName(), events); return 1; } JNIEnv* env = AndroidRuntime::getJNIEnv(); - status_t status = r->consumeEvents(env, false /*consumeBatches*/, -1); - r->mMessageQueue->raiseAndClearException(env, "handleReceiveCallback"); + status_t status = consumeEvents(env, false /*consumeBatches*/, -1); + mMessageQueue->raiseAndClearException(env, "handleReceiveCallback"); return status == OK || status == NO_MEMORY ? 1 : 0; } @@ -256,6 +256,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj, static void nativeDispose(JNIEnv* env, jclass clazz, jint receiverPtr) { sp receiver = reinterpret_cast(receiverPtr); + receiver->dispose(); receiver->decStrong(gInputEventReceiverClassInfo.clazz); // drop reference held by the object } -- cgit v1.1