Avoid deadlock in MessageQueue::onNewVsyncSchedule
It is hard to reproduce, but if the
VSyncDispatchTimerQueueEntry::callback is called at just the right time,
it will set mRunning to true, then wait on MessageQueue's mVsync.mutex,
which was already locked by onNewVsyncSchedule. When
onNewVsyncScheduleLocked destructs the old VSyncCallbackRegistration, it
needs to wait until mRunning is set back to false. But
VSyncDispatchTimerQueueEntry::callback cannot do that until
MessageQueue::vsyncCallback can lock the mutex.
Avoid this deadlock by moving the old VSyncCallbackRegistration's
destruction until after mVsync.mutex is released.
Bug: 276367387
Test: infeasible
Change-Id: I86e66df59c64e81c4aa721a25250697f61237488
diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h
index 9c9b2f3..a523147 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.h
+++ b/services/surfaceflinger/Scheduler/MessageQueue.h
@@ -117,9 +117,9 @@
struct Vsync {
frametimeline::TokenManager* tokenManager = nullptr;
- std::unique_ptr<scheduler::VSyncCallbackRegistration> registration;
mutable std::mutex mutex;
+ std::unique_ptr<scheduler::VSyncCallbackRegistration> registration GUARDED_BY(mutex);
TracedOrdinal<std::chrono::nanoseconds> workDuration
GUARDED_BY(mutex) = {"VsyncWorkDuration-sf", std::chrono::nanoseconds(0)};
TimePoint lastCallbackTime GUARDED_BY(mutex);
@@ -129,7 +129,10 @@
Vsync mVsync;
- void onNewVsyncScheduleLocked(std::shared_ptr<scheduler::VSyncDispatch>) REQUIRES(mVsync.mutex);
+ // Returns the old registration so it can be destructed outside the lock to
+ // avoid deadlock.
+ std::unique_ptr<scheduler::VSyncCallbackRegistration> onNewVsyncScheduleLocked(
+ std::shared_ptr<scheduler::VSyncDispatch>) REQUIRES(mVsync.mutex);
public:
explicit MessageQueue(ICompositor&);