hwc3: Make the ComposerCommandEngine as local variable

A race condition exists because HWC uses only one
ComposerCommandEngine::Writer for all displays.
The writer takes responsibility to package AIDL functions’ return
values and deliver the package to SF. When a thread is packaging
the return value of the inner display by Writer, another
thread may call Writer::reset() for the outer display.

Bug: 247074458
Test: switch applications
Change-Id: I7393bd6d01008b09ac4f4a733fc7d024aaf05352

4 files changed, 16 insertions, 19 deletions

diff --git a/hwc3/ComposerClient.cpp b/hwc3/ComposerClient.cpp
index 7324e5c..7a3c72a 100644
--- a/hwc3/ComposerClient.cpp
+++ b/hwc3/ComposerClient.cpp
@@ -33,25 +33,11 @@ bool ComposerClient::init() {
         return false;
     }
 
-    mCommandEngine = std::make_unique<ComposerCommandEngine>(mHal, mResources.get());
-    if (mCommandEngine == nullptr) {
-        return false;
-    }
-    if (!mCommandEngine->init()) {
-        mCommandEngine = nullptr;
-        return false;
-    }
-
     return true;
 }
 
 ComposerClient::~ComposerClient() {
     DEBUG_FUNC();
-    // not initialized
-    if (!mCommandEngine) {
-        return;
-    }
-
     LOG(DEBUG) << "destroying composer client";
 
     mHal->unregisterEventCallback();
@@ -111,7 +97,19 @@ ndk::ScopedAStatus ComposerClient::destroyVirtualDisplay(int64_t display) {
 ndk::ScopedAStatus ComposerClient::executeCommands(const std::vector<DisplayCommand>& commands,
                                                    std::vector<CommandResultPayload>* results) {
     DEBUG_FUNC();
-    auto err = mCommandEngine->execute(commands, results);
+    ComposerCommandEngine engine(mHal, mResources.get());
+
+    auto err = engine.init();
+    if (err != ::android::NO_ERROR) {
+        LOG(ERROR) << "executeCommands(): init ComposerCommandEngine failed " << err;
+        return TO_BINDER_STATUS(err);
+    }
+
+    err = engine.execute(commands, results);
+    if (err != ::android::NO_ERROR) {
+        LOG(ERROR) << "executeCommands(): execute failed " << err;
+        return TO_BINDER_STATUS(err);
+    }
     return TO_BINDER_STATUS(err);
 }
 
diff --git a/hwc3/ComposerClient.h b/hwc3/ComposerClient.h
index e10e3b6..9e85396 100644
--- a/hwc3/ComposerClient.h
+++ b/hwc3/ComposerClient.h
@@ -136,7 +136,6 @@ private:
 
     IComposerHal* mHal;
     std::unique_ptr<IResourceManager> mResources;
-    std::unique_ptr<ComposerCommandEngine> mCommandEngine;
     std::function<void()> mOnClientDestroyed;
     std::unique_ptr<HalEventCallback> mHalEventCallback;
 };
diff --git a/hwc3/ComposerCommandEngine.cpp b/hwc3/ComposerCommandEngine.cpp
index 4b3a1a3..4dac2df 100644
--- a/hwc3/ComposerCommandEngine.cpp
+++ b/hwc3/ComposerCommandEngine.cpp
@@ -55,9 +55,9 @@ namespace aidl::android::hardware::graphics::composer3::impl {
         }                                                                         \
     } while (0)
 
-bool ComposerCommandEngine::init() {
+int32_t ComposerCommandEngine::init() {
     mWriter = std::make_unique<ComposerServiceWriter>();
-    return (mWriter != nullptr);
+    return (mWriter != nullptr) ? ::android::NO_ERROR : ::android::NO_MEMORY;
 }
 
 int32_t ComposerCommandEngine::execute(const std::vector<DisplayCommand>& commands,
diff --git a/hwc3/ComposerCommandEngine.h b/hwc3/ComposerCommandEngine.h
index ae68285..872c7e5 100644
--- a/hwc3/ComposerCommandEngine.h
+++ b/hwc3/ComposerCommandEngine.h
@@ -30,7 +30,7 @@ class ComposerCommandEngine {
   public:
       ComposerCommandEngine(IComposerHal* hal, IResourceManager* resources)
             : mHal(hal), mResources(resources) {}
-      bool init();
+      int32_t init();
 
       int32_t execute(const std::vector<DisplayCommand>& commands,
                       std::vector<CommandResultPayload>* result);