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

Commit b056f98c authored by Yisroel Forta's avatar Yisroel Forta
Browse files

Remove lock from send start timestamps

It can occur that doDie and surfaceflinger callback occur
simultaneously, this will result in doDie holding the
ViewRootImpl lock while it waits for dequeueBuffer but
maybeSendAppStartTimes will be blocking the thread
waiting on the same lock causing a deadlock.

To remedy this, replace the lock with an atomic variable
for mAppStartTimestampsSent and post to main thread in
all cases. The synchronization only exists to ensure that
sending timestamp is not triggered multiple times
simultaneously, so this is sufficient. Post to main thread
in all cases to ensure values are updated.

Test: AppStartTests, no way to test the actual deadlock
Bug: 359627195
Bug: 357735727
Bug: 357795880
Flag: NONE infeasible
Change-Id: I9c1f7b677b8db93a9ec5a54cf7d0aa9dd9100bb8
parent e4ec8c40
Loading
Loading
Loading
Loading
+24 −37
Original line number Diff line number Diff line
@@ -303,6 +303,7 @@ import java.util.OptionalInt;
import java.util.Queue;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.Predicate;
/**
@@ -1200,8 +1201,7 @@ public final class ViewRootImpl implements ViewParent,
    private String mLargestViewTraceName;
    private final boolean mAppStartInfoTimestampsFlagValue;
    @GuardedBy("this")
    private boolean mAppStartTimestampsSent = false;
    private AtomicBoolean mAppStartTimestampsSent = new AtomicBoolean(false);
    private boolean mAppStartTrackingStarted = false;
    private long mRenderThreadDrawStartTimeNs = -1;
    private long mFirstFramePresentedTimeNs = -1;
@@ -2646,7 +2646,7 @@ public final class ViewRootImpl implements ViewParent,
                destroySurface();
                // Reset so they can be sent again for warm starts.
                mAppStartTimestampsSent = false;
                mAppStartTimestampsSent.set(false);
                mAppStartTrackingStarted = false;
                mRenderThreadDrawStartTimeNs = -1;
                mFirstFramePresentedTimeNs = -1;
@@ -4502,43 +4502,30 @@ public final class ViewRootImpl implements ViewParent,
    }
    private void maybeSendAppStartTimes() {
        synchronized (this) {
            if (mAppStartTimestampsSent) {
        if (mAppStartTimestampsSent.get()) {
            // Don't send timestamps more than once.
            return;
        }
            // If we already have {@link mRenderThreadDrawStartTimeNs} then pass it through, if not
            // post to main thread and check if we have it there.
            if (mRenderThreadDrawStartTimeNs != -1) {
                sendAppStartTimesLocked();
            } else {
        // Post to main thread
        mHandler.post(new Runnable() {
            @Override
            public void run() {
                        synchronized (ViewRootImpl.this) {
                if (mRenderThreadDrawStartTimeNs == -1) {
                    return;
                }
                            sendAppStartTimesLocked();
                        }
                    }
                });
            }
        }
    }
    @GuardedBy("this")
    private void sendAppStartTimesLocked() {
                try {
                    ActivityManager.getService().reportStartInfoViewTimestamps(
                            mRenderThreadDrawStartTimeNs, mFirstFramePresentedTimeNs);
            mAppStartTimestampsSent = true;
                    mAppStartTimestampsSent.set(true);
                } catch (RemoteException e) {
                    // Ignore, timestamps may be lost.
                    if (DBG) Log.d(TAG, "Exception attempting to report start timestamps.", e);
                }
            }
        });
    }
    /**
     * Helper used to notify the service to block projection when a sensitive