Skip to content

Commit 6e8e41a

Browse files
Jeff BrownAndroid (Google) Code Review
authored andcommitted
Merge "Use sp<LooperCallback> to fix race causing dangling pointer." into jb-dev
2 parents 53913ed + 80a1de1 commit 6e8e41a

File tree

2 files changed

+40
-37
lines changed

2 files changed

+40
-37
lines changed

core/jni/android_view_DisplayEventReceiver.cpp

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,13 @@ static struct {
4242
} gDisplayEventReceiverClassInfo;
4343

4444

45-
class NativeDisplayEventReceiver : public RefBase {
45+
class NativeDisplayEventReceiver : public LooperCallback {
4646
public:
4747
NativeDisplayEventReceiver(JNIEnv* env,
4848
jobject receiverObj, const sp<MessageQueue>& messageQueue);
4949

5050
status_t initialize();
51+
void dispose();
5152
status_t scheduleVsync();
5253

5354
protected:
@@ -59,7 +60,7 @@ class NativeDisplayEventReceiver : public RefBase {
5960
DisplayEventReceiver mReceiver;
6061
bool mWaitingForVsync;
6162

62-
static int handleReceiveCallback(int receiveFd, int events, void* data);
63+
virtual int handleEvent(int receiveFd, int events, void* data);
6364
bool readLastVsyncMessage(nsecs_t* outTimestamp, uint32_t* outCount);
6465
};
6566

@@ -72,12 +73,6 @@ NativeDisplayEventReceiver::NativeDisplayEventReceiver(JNIEnv* env,
7273
}
7374

7475
NativeDisplayEventReceiver::~NativeDisplayEventReceiver() {
75-
ALOGV("receiver %p ~ Disposing display event receiver.", this);
76-
77-
if (!mReceiver.initCheck()) {
78-
mMessageQueue->getLooper()->removeFd(mReceiver.getFd());
79-
}
80-
8176
JNIEnv* env = AndroidRuntime::getJNIEnv();
8277
env->DeleteGlobalRef(mReceiverObjGlobal);
8378
}
@@ -90,13 +85,21 @@ status_t NativeDisplayEventReceiver::initialize() {
9085
}
9186

9287
int rc = mMessageQueue->getLooper()->addFd(mReceiver.getFd(), 0, ALOOPER_EVENT_INPUT,
93-
handleReceiveCallback, this);
88+
this, NULL);
9489
if (rc < 0) {
9590
return UNKNOWN_ERROR;
9691
}
9792
return OK;
9893
}
9994

95+
void NativeDisplayEventReceiver::dispose() {
96+
ALOGV("receiver %p ~ Disposing display event receiver.", this);
97+
98+
if (!mReceiver.initCheck()) {
99+
mMessageQueue->getLooper()->removeFd(mReceiver.getFd());
100+
}
101+
}
102+
100103
status_t NativeDisplayEventReceiver::scheduleVsync() {
101104
if (!mWaitingForVsync) {
102105
ALOGV("receiver %p ~ Scheduling vsync.", this);
@@ -117,9 +120,7 @@ status_t NativeDisplayEventReceiver::scheduleVsync() {
117120
return OK;
118121
}
119122

120-
int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events, void* data) {
121-
sp<NativeDisplayEventReceiver> r = static_cast<NativeDisplayEventReceiver*>(data);
122-
123+
int NativeDisplayEventReceiver::handleEvent(int receiveFd, int events, void* data) {
123124
if (events & (ALOOPER_EVENT_ERROR | ALOOPER_EVENT_HANGUP)) {
124125
ALOGE("Display event receiver pipe was closed or an error occurred. "
125126
"events=0x%x", events);
@@ -135,23 +136,23 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
135136
// Drain all pending events, keep the last vsync.
136137
nsecs_t vsyncTimestamp;
137138
uint32_t vsyncCount;
138-
if (!r->readLastVsyncMessage(&vsyncTimestamp, &vsyncCount)) {
139-
ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", data);
139+
if (!readLastVsyncMessage(&vsyncTimestamp, &vsyncCount)) {
140+
ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", this);
140141
return 1; // keep the callback, did not obtain a vsync pulse
141142
}
142143

143144
ALOGV("receiver %p ~ Vsync pulse: timestamp=%lld, count=%d",
144-
data, vsyncTimestamp, vsyncCount);
145-
r->mWaitingForVsync = false;
145+
this, vsyncTimestamp, vsyncCount);
146+
mWaitingForVsync = false;
146147

147148
JNIEnv* env = AndroidRuntime::getJNIEnv();
148149

149-
ALOGV("receiver %p ~ Invoking vsync handler.", data);
150-
env->CallVoidMethod(r->mReceiverObjGlobal,
150+
ALOGV("receiver %p ~ Invoking vsync handler.", this);
151+
env->CallVoidMethod(mReceiverObjGlobal,
151152
gDisplayEventReceiverClassInfo.dispatchVsync, vsyncTimestamp, vsyncCount);
152-
ALOGV("receiver %p ~ Returned from vsync handler.", data);
153+
ALOGV("receiver %p ~ Returned from vsync handler.", this);
153154

154-
r->mMessageQueue->raiseAndClearException(env, "dispatchVsync");
155+
mMessageQueue->raiseAndClearException(env, "dispatchVsync");
155156
return 1; // keep the callback
156157
}
157158

@@ -201,6 +202,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,
201202
static void nativeDispose(JNIEnv* env, jclass clazz, jint receiverPtr) {
202203
sp<NativeDisplayEventReceiver> receiver =
203204
reinterpret_cast<NativeDisplayEventReceiver*>(receiverPtr);
205+
receiver->dispose();
204206
receiver->decStrong(gDisplayEventReceiverClassInfo.clazz); // drop reference held by the object
205207
}
206208

core/jni/android_view_InputEventReceiver.cpp

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ static struct {
4444
} gInputEventReceiverClassInfo;
4545

4646

47-
class NativeInputEventReceiver : public RefBase {
47+
class NativeInputEventReceiver : public LooperCallback {
4848
public:
4949
NativeInputEventReceiver(JNIEnv* env,
5050
jobject receiverObj, const sp<InputChannel>& inputChannel,
5151
const sp<MessageQueue>& messageQueue);
5252

5353
status_t initialize();
54+
void dispose();
5455
status_t finishInputEvent(uint32_t seq, bool handled);
5556
status_t consumeEvents(JNIEnv* env, bool consumeBatches, nsecs_t frameTime);
5657

@@ -68,7 +69,7 @@ class NativeInputEventReceiver : public RefBase {
6869
return mInputConsumer.getChannel()->getName().string();
6970
}
7071

71-
static int handleReceiveCallback(int receiveFd, int events, void* data);
72+
virtual int handleEvent(int receiveFd, int events, void* data);
7273
};
7374

7475

@@ -84,23 +85,24 @@ NativeInputEventReceiver::NativeInputEventReceiver(JNIEnv* env,
8485
}
8586

8687
NativeInputEventReceiver::~NativeInputEventReceiver() {
87-
#if DEBUG_DISPATCH_CYCLE
88-
ALOGD("channel '%s' ~ Disposing input event receiver.", getInputChannelName());
89-
#endif
90-
91-
mMessageQueue->getLooper()->removeFd(mInputConsumer.getChannel()->getFd());
92-
9388
JNIEnv* env = AndroidRuntime::getJNIEnv();
9489
env->DeleteGlobalRef(mReceiverObjGlobal);
9590
}
9691

9792
status_t NativeInputEventReceiver::initialize() {
9893
int receiveFd = mInputConsumer.getChannel()->getFd();
99-
mMessageQueue->getLooper()->addFd(
100-
receiveFd, 0, ALOOPER_EVENT_INPUT, handleReceiveCallback, this);
94+
mMessageQueue->getLooper()->addFd(receiveFd, 0, ALOOPER_EVENT_INPUT, this, NULL);
10195
return OK;
10296
}
10397

98+
void NativeInputEventReceiver::dispose() {
99+
#if DEBUG_DISPATCH_CYCLE
100+
ALOGD("channel '%s' ~ Disposing input event receiver.", getInputChannelName());
101+
#endif
102+
103+
mMessageQueue->getLooper()->removeFd(mInputConsumer.getChannel()->getFd());
104+
}
105+
104106
status_t NativeInputEventReceiver::finishInputEvent(uint32_t seq, bool handled) {
105107
#if DEBUG_DISPATCH_CYCLE
106108
ALOGD("channel '%s' ~ Finished input event.", getInputChannelName());
@@ -114,24 +116,22 @@ status_t NativeInputEventReceiver::finishInputEvent(uint32_t seq, bool handled)
114116
return status;
115117
}
116118

117-
int NativeInputEventReceiver::handleReceiveCallback(int receiveFd, int events, void* data) {
118-
sp<NativeInputEventReceiver> r = static_cast<NativeInputEventReceiver*>(data);
119-
119+
int NativeInputEventReceiver::handleEvent(int receiveFd, int events, void* data) {
120120
if (events & (ALOOPER_EVENT_ERROR | ALOOPER_EVENT_HANGUP)) {
121121
ALOGE("channel '%s' ~ Publisher closed input channel or an error occurred. "
122-
"events=0x%x", r->getInputChannelName(), events);
122+
"events=0x%x", getInputChannelName(), events);
123123
return 0; // remove the callback
124124
}
125125

126126
if (!(events & ALOOPER_EVENT_INPUT)) {
127127
ALOGW("channel '%s' ~ Received spurious callback for unhandled poll event. "
128-
"events=0x%x", r->getInputChannelName(), events);
128+
"events=0x%x", getInputChannelName(), events);
129129
return 1;
130130
}
131131

132132
JNIEnv* env = AndroidRuntime::getJNIEnv();
133-
status_t status = r->consumeEvents(env, false /*consumeBatches*/, -1);
134-
r->mMessageQueue->raiseAndClearException(env, "handleReceiveCallback");
133+
status_t status = consumeEvents(env, false /*consumeBatches*/, -1);
134+
mMessageQueue->raiseAndClearException(env, "handleReceiveCallback");
135135
return status == OK || status == NO_MEMORY ? 1 : 0;
136136
}
137137

@@ -256,6 +256,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,
256256
static void nativeDispose(JNIEnv* env, jclass clazz, jint receiverPtr) {
257257
sp<NativeInputEventReceiver> receiver =
258258
reinterpret_cast<NativeInputEventReceiver*>(receiverPtr);
259+
receiver->dispose();
259260
receiver->decStrong(gInputEventReceiverClassInfo.clazz); // drop reference held by the object
260261
}
261262

0 commit comments

Comments
 (0)