Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a9daa164 authored by Jeff Brown's avatar Jeff Brown Committed by Android (Google) Code Review
Browse files

Merge "Fix possible races in vsync infrastructure."

parents b35b9a86 58aedbc9
Loading
Loading
Loading
Loading
+72 −7
Original line number 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
 * 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
 */
public final class Choreographer extends Handler {
    private static final String TAG = "Choreographer";
    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.
    // 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
@@ -78,6 +86,8 @@ public final class Choreographer extends Handler {

    private static final int MSG_DO_ANIMATION = 0;
    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();

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

    private boolean mAnimationScheduled;
    private boolean mDrawScheduled;
    private boolean mFrameDisplayEventReceiverNeeded;
    private FrameDisplayEventReceiver mFrameDisplayEventReceiver;
    private long mLastAnimationTime;
    private long mLastDrawTime;
@@ -158,10 +169,21 @@ public final class Choreographer extends Handler {
                if (DEBUG) {
                    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 {
                final long now = SystemClock.uptimeMillis();
                final long nextAnimationTime = Math.max(mLastAnimationTime + sFrameDelay, now);
@@ -224,6 +246,12 @@ public final class Choreographer extends Handler {
            case MSG_DO_DRAW:
                doDraw();
                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.
     *
@@ -386,7 +438,9 @@ public final class Choreographer extends Handler {

            if (mAnimationScheduled) {
                mAnimationScheduled = false;
                if (!USE_VSYNC) {
                if (USE_VSYNC) {
                    removeMessages(MSG_DO_SCHEDULE_VSYNC);
                } else {
                    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) {
                mFrameDisplayEventReceiver.dispose();
                mFrameDisplayEventReceiver = null;
                    sendEmptyMessageDelayed(MSG_DO_DISPOSE_RECEIVER, DISPOSE_RECEIVER_DELAY);
                }
            }
        }
    }

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

    /**
     * Listens for animation frame timing events.
     */
+4 −0
Original line number 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
 * 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
 */
public abstract class DisplayEventReceiver {
+13 −25
Original line number Diff line number Diff line
@@ -59,21 +59,20 @@ private:
    sp<Looper> mLooper;
    DisplayEventReceiver mReceiver;
    bool mWaitingForVsync;
    bool mFdCallbackRegistered;
};


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

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

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

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

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

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

@@ -150,7 +144,7 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
    DisplayEventReceiver::Event buf[EVENT_BUFFER_SIZE];
    ssize_t n;
    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) {
            if (buf[n].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
                vsyncTimestamp = buf[n].header.timestamp;
@@ -161,20 +155,20 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
    }

    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
    }

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

    JNIEnv* env = AndroidRuntime::getJNIEnv();

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

    if (env->ExceptionCheck()) {
        ALOGE("An exception occurred while dispatching a vsync event.");
@@ -182,14 +176,8 @@ int NativeDisplayEventReceiver::handleReceiveCallback(int receiveFd, int events,
        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
}
    r->mFdCallbackRegistered = false;
    return 0; // remove the callback
}


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