Fix possible races in vsync infrastructure. (58aedbc9) · Commits · e / os / android_frameworks_base

core/java/android/view/Choreographer.java

+72 −7

Original line number	Original line	Diff line number	Diff line
	@@ -31,12 +31,20 @@ import android.util.Log;
	* This object is thread-safe. Other threads can add and remove listeners		* This object is thread-safe. Other threads can add and remove listeners
	* or schedule work to occur at a later time on the UI thread.		* or schedule work to occur at a later time on the UI thread.
	*		*
			* Ensuring thread-safety is a little tricky because the {@link DisplayEventReceiver}
			* can only be accessed from the UI thread so operations that touch the event receiver
			* are posted to the UI thread if needed.
			*
	* @hide		* @hide
	*/		*/
	public final class Choreographer extends Handler {		public final class Choreographer extends Handler {
	private static final String TAG = "Choreographer";		private static final String TAG = "Choreographer";
	private static final boolean DEBUG = false;		private static final boolean DEBUG = false;

			// Amount of time in ms to wait before actually disposing of the display event
			// receiver after all listeners have been removed.
			private static final long DISPOSE_RECEIVER_DELAY = 200;

	// The default amount of time in ms between animation frames.		// The default amount of time in ms between animation frames.
	// When vsync is not enabled, we want to have some idea of how long we should		// When vsync is not enabled, we want to have some idea of how long we should
	// wait before posting the next animation message. It is important that the		// wait before posting the next animation message. It is important that the
	@@ -78,6 +86,8 @@ public final class Choreographer extends Handler {

	private static final int MSG_DO_ANIMATION = 0;		private static final int MSG_DO_ANIMATION = 0;
	private static final int MSG_DO_DRAW = 1;		private static final int MSG_DO_DRAW = 1;
			private static final int MSG_DO_SCHEDULE_VSYNC = 2;
			private static final int MSG_DO_DISPOSE_RECEIVER = 3;

	private final Object mLock = new Object();		private final Object mLock = new Object();

	@@ -88,6 +98,7 @@ public final class Choreographer extends Handler {

	private boolean mAnimationScheduled;		private boolean mAnimationScheduled;
	private boolean mDrawScheduled;		private boolean mDrawScheduled;
			private boolean mFrameDisplayEventReceiverNeeded;
	private FrameDisplayEventReceiver mFrameDisplayEventReceiver;		private FrameDisplayEventReceiver mFrameDisplayEventReceiver;
	private long mLastAnimationTime;		private long mLastAnimationTime;
	private long mLastDrawTime;		private long mLastDrawTime;
	@@ -158,10 +169,21 @@ public final class Choreographer extends Handler {
	if (DEBUG) {		if (DEBUG) {
	Log.d(TAG, "Scheduling vsync for animation.");		Log.d(TAG, "Scheduling vsync for animation.");
	}		}
	if (mFrameDisplayEventReceiver == null) {
	mFrameDisplayEventReceiver = new FrameDisplayEventReceiver(mLooper);		// If running on the Looper thread, then schedule the vsync immediately,
			// otherwise post a message to schedule the vsync from the UI thread
			// as soon as possible.
			if (!mFrameDisplayEventReceiverNeeded) {
			mFrameDisplayEventReceiverNeeded = true;
			if (mFrameDisplayEventReceiver != null) {
			removeMessages(MSG_DO_DISPOSE_RECEIVER);
			}
			}
			if (isRunningOnLooperThreadLocked()) {
			doScheduleVsyncLocked();
			} else {
			sendMessageAtFrontOfQueue(obtainMessage(MSG_DO_SCHEDULE_VSYNC));
	}		}
	mFrameDisplayEventReceiver.scheduleVsync();
	} else {		} else {
	final long now = SystemClock.uptimeMillis();		final long now = SystemClock.uptimeMillis();
	final long nextAnimationTime = Math.max(mLastAnimationTime + sFrameDelay, now);		final long nextAnimationTime = Math.max(mLastAnimationTime + sFrameDelay, now);
	@@ -224,6 +246,12 @@ public final class Choreographer extends Handler {
	case MSG_DO_DRAW:		case MSG_DO_DRAW:
	doDraw();		doDraw();
	break;		break;
			case MSG_DO_SCHEDULE_VSYNC:
			doScheduleVsync();
			break;
			case MSG_DO_DISPOSE_RECEIVER:
			doDisposeReceiver();
			break;
	}		}
	}		}

	@@ -295,6 +323,30 @@ public final class Choreographer extends Handler {
	}		}
	}		}

			private void doScheduleVsync() {
			synchronized (mLock) {
			doScheduleVsyncLocked();
			}
			}

			private void doScheduleVsyncLocked() {
			if (mFrameDisplayEventReceiverNeeded && mAnimationScheduled) {
			if (mFrameDisplayEventReceiver == null) {
			mFrameDisplayEventReceiver = new FrameDisplayEventReceiver(mLooper);
			}
			mFrameDisplayEventReceiver.scheduleVsync();
			}
			}

			private void doDisposeReceiver() {
			synchronized (mLock) {
			if (!mFrameDisplayEventReceiverNeeded && mFrameDisplayEventReceiver != null) {
			mFrameDisplayEventReceiver.dispose();
			mFrameDisplayEventReceiver = null;
			}
			}
			}

	/**		/**
	* Adds an animation listener.		* Adds an animation listener.
	*		*
	@@ -386,7 +438,9 @@ public final class Choreographer extends Handler {

	if (mAnimationScheduled) {		if (mAnimationScheduled) {
	mAnimationScheduled = false;		mAnimationScheduled = false;
	if (!USE_VSYNC) {		if (USE_VSYNC) {
			removeMessages(MSG_DO_SCHEDULE_VSYNC);
			} else {
	removeMessages(MSG_DO_ANIMATION);		removeMessages(MSG_DO_ANIMATION);
	}		}
	}		}
	@@ -398,13 +452,24 @@ public final class Choreographer extends Handler {
	}		}
	}		}

			// Post a message to dispose the display event receiver if we haven't needed
			// it again after a certain amount of time has elapsed. Another reason to
			// defer disposal is that it is possible for use to attempt to dispose the
			// receiver while handling a vsync event that it dispatched, which might
			// cause a few problems...
			if (mFrameDisplayEventReceiverNeeded) {
			mFrameDisplayEventReceiverNeeded = false;
	if (mFrameDisplayEventReceiver != null) {		if (mFrameDisplayEventReceiver != null) {
	mFrameDisplayEventReceiver.dispose();		sendEmptyMessageDelayed(MSG_DO_DISPOSE_RECEIVER, DISPOSE_RECEIVER_DELAY);
	mFrameDisplayEventReceiver = null;		}
	}		}
	}		}
	}		}

			private boolean isRunningOnLooperThreadLocked() {
			return Looper.myLooper() == mLooper;
			}

	/**		/**
	* Listens for animation frame timing events.		* Listens for animation frame timing events.
	*/		*/

core/java/android/view/DisplayEventReceiver.java

+4 −0

Original line number	Original line	Diff line number	Diff line
	@@ -25,6 +25,10 @@ import android.util.Log;
	/**		/**
	* Provides a low-level mechanism for an application to receive display events		* Provides a low-level mechanism for an application to receive display events
	* such as vertical sync.		* such as vertical sync.
			*
			* The display event receive is NOT thread safe. Moreover, its methods must only
			* be called on the Looper thread to which it is attached.
			*
	* @hide		* @hide
	*/		*/
	public abstract class DisplayEventReceiver {		public abstract class DisplayEventReceiver {

core/jni/android_view_DisplayEventReceiver.cpp

+13 −25

Original line number	Original line	Diff line number	Diff line
	@@ -59,21 +59,20 @@ private:
	sp<Looper> mLooper;		sp<Looper> mLooper;
	DisplayEventReceiver mReceiver;		DisplayEventReceiver mReceiver;
	bool mWaitingForVsync;		bool mWaitingForVsync;
	bool mFdCallbackRegistered;
	};		};


	NativeDisplayEventReceiver::NativeDisplayEventReceiver(JNIEnv* env,		NativeDisplayEventReceiver::NativeDisplayEventReceiver(JNIEnv* env,
	jobject receiverObj, const sp<Looper>& looper) :		jobject receiverObj, const sp<Looper>& looper) :
	mReceiverObjGlobal(env->NewGlobalRef(receiverObj)),		mReceiverObjGlobal(env->NewGlobalRef(receiverObj)),
	mLooper(looper), mWaitingForVsync(false), mFdCallbackRegistered(false) {		mLooper(looper), mWaitingForVsync(false) {
	ALOGV("receiver %p ~ Initializing input event receiver.", this);		ALOGV("receiver %p ~ Initializing input event receiver.", this);
	}		}

	NativeDisplayEventReceiver::~NativeDisplayEventReceiver() {		NativeDisplayEventReceiver::~NativeDisplayEventReceiver() {
	ALOGV("receiver %p ~ Disposing display event receiver.", this);		ALOGV("receiver %p ~ Disposing display event receiver.", this);

	if (mFdCallbackRegistered) {		if (!mReceiver.initCheck()) {
	mLooper->removeFd(mReceiver.getFd());		mLooper->removeFd(mReceiver.getFd());
	}		}

	@@ -88,6 +87,11 @@ status_t NativeDisplayEventReceiver::initialize() {
	return result;		return result;
	}		}

			int rc = mLooper->addFd(mReceiver.getFd(), 0, ALOOPER_EVENT_INPUT,
			handleReceiveCallback, this);
			if (rc < 0) {
			return UNKNOWN_ERROR;
			}
	return OK;		return OK;
	}		}

	@@ -113,15 +117,6 @@ status_t NativeDisplayEventReceiver::scheduleVsync() {
	return status;		return status;
	}		}

	if (!mFdCallbackRegistered) {
	int rc = mLooper->addFd(mReceiver.getFd(), 0, ALOOPER_EVENT_INPUT,
	handleReceiveCallback, this);
	if (rc < 0) {
	return UNKNOWN_ERROR;
	}
	mFdCallbackRegistered = true;
	}

	mWaitingForVsync = true;		mWaitingForVsync = true;
	}		}
	return OK;		return OK;
	@@ -133,7 +128,6 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
	if (events & (ALOOPER_EVENT_ERROR \| ALOOPER_EVENT_HANGUP)) {		if (events & (ALOOPER_EVENT_ERROR \| ALOOPER_EVENT_HANGUP)) {
	ALOGE("Display event receiver pipe was closed or an error occurred. "		ALOGE("Display event receiver pipe was closed or an error occurred. "
	"events=0x%x", events);		"events=0x%x", events);
	r->mFdCallbackRegistered = false;
	return 0; // remove the callback		return 0; // remove the callback
	}		}

	@@ -150,7 +144,7 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
	DisplayEventReceiver::Event buf[EVENT_BUFFER_SIZE];		DisplayEventReceiver::Event buf[EVENT_BUFFER_SIZE];
	ssize_t n;		ssize_t n;
	while ((n = r->mReceiver.getEvents(buf, EVENT_BUFFER_SIZE)) > 0) {		while ((n = r->mReceiver.getEvents(buf, EVENT_BUFFER_SIZE)) > 0) {
	ALOGV("receiver %p ~ Read %d events.", this, int(n));		ALOGV("receiver %p ~ Read %d events.", data, int(n));
	while (n-- > 0) {		while (n-- > 0) {
	if (buf[n].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {		if (buf[n].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
	vsyncTimestamp = buf[n].header.timestamp;		vsyncTimestamp = buf[n].header.timestamp;
	@@ -161,20 +155,20 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
	}		}

	if (vsyncTimestamp < 0) {		if (vsyncTimestamp < 0) {
	ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", this);		ALOGV("receiver %p ~ Woke up but there was no vsync pulse!", data);
	return 1; // keep the callback, did not obtain a vsync pulse		return 1; // keep the callback, did not obtain a vsync pulse
	}		}

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

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

	ALOGV("receiver %p ~ Invoking vsync handler.", this);		ALOGV("receiver %p ~ Invoking vsync handler.", data);
	env->CallVoidMethod(r->mReceiverObjGlobal,		env->CallVoidMethod(r->mReceiverObjGlobal,
	gDisplayEventReceiverClassInfo.dispatchVsync, vsyncTimestamp, vsyncCount);		gDisplayEventReceiverClassInfo.dispatchVsync, vsyncTimestamp, vsyncCount);
	ALOGV("receiver %p ~ Returned from vsync handler.", this);		ALOGV("receiver %p ~ Returned from vsync handler.", data);

	if (env->ExceptionCheck()) {		if (env->ExceptionCheck()) {
	ALOGE("An exception occurred while dispatching a vsync event.");		ALOGE("An exception occurred while dispatching a vsync event.");
	@@ -182,14 +176,8 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
	env->ExceptionClear();		env->ExceptionClear();
	}		}

	// Check whether dispatchVsync called scheduleVsync reentrantly and set mWaitingForVsync.
	// If so, keep the callback, otherwise remove it.
	if (r->mWaitingForVsync) {
	return 1; // keep the callback		return 1; // keep the callback
	}		}
	r->mFdCallbackRegistered = false;
	return 0; // remove the callback
	}


	static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,		static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,