drm_hwcomposer: Mitigate race condition in HwcDisplay destructor
There is a race between the main thread and vsync thread when tearing
down the HwcDisplay.
In the HwcDisplay destructor, Deinit is called, which will call
StopThread on the vsync worker, and release the VSyncWorker reference.
The main thread is holding the main lock, and the VSyncWorker thread may
be waiting on the main lock in the out_event callback.
After the HwcDisplay destructor is complete, the main thread will
eventually release the main lock. At this point, the vsync thread can
wake and will have a dangling pointer to 'this'.
This can be mitigated by:
- Explicitly set the HwcDisplay pipeline to nullptr, which will stop the
vsync thread.
- Release the main lock after setting the pipeline to nullptr and sleep,
to allow the vsync thread to complete
A more robust solution would be to provide a mechanism to ensure that
the HwcDisplay knows that the vsync thread has completed before the
HwcDisplay has been destructed.
Change-Id: I79626427a94330b15bc138dad3163d0f2d934466
Signed-off-by: Drew Davenport <ddavenport@google.com>
diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index e60d9e8..e12a236 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -79,6 +79,20 @@
ComposerClient::~ComposerClient() {
DEBUG_FUNC();
+ {
+ // First Deinit the displays to start shutting down the Display's dependent
+ // threads such as VSyncWorker.
+ const std::unique_lock lock(hwc_->GetResMan().GetMainLock());
+ hwc_->DeinitDisplays();
+ }
+ // Sleep to wait for threads to complete and exit.
+ const int time_for_threads_to_exit_us = 200000;
+ usleep(time_for_threads_to_exit_us);
+ {
+ // Hold the lock while destructing the hwc_ and the objects that it owns.
+ const std::unique_lock lock(hwc_->GetResMan().GetMainLock());
+ hwc_.reset();
+ }
LOG(DEBUG) << "removed composer client";
}