commit 8aed5d2b7b621849e66cb638951df4de301c9a12
author Patrick Williams <pdwilliams@google.com> Mon Oct 31 22:18:10 2022 +0000
committer Patrick Williams <pdwilliams@google.com> Thu Nov 03 19:17:25 2022 +0000
tree 81213dc79a38f2150013a08eec5a65e251d49b3d
parent d500ef2e8a803528652fd1c002944cc001f52783

RE: move useProtectedContext calls to drawLayers

This change removes a race condition when calling RenderEngine from
multiple threads. Before this change, two competing threads could call
RenderEngine::useProtectedContext with different values, potentially
causing RenderEngine::drawLayers to use the wrong context.

We currently call RenderEngine from a single thread but that will change
with the upcoming refactor of SurfaceFlinger::renderScreenImpl.

Bug: 238643986
Test: presubmits
Change-Id: I78ca00797b1604cedae1f134e963b6b78f2b802f

libs/renderengine/threaded/RenderEngineThreaded.cpp

diff --git a/libs/renderengine/threaded/RenderEngineThreaded.cpp b/libs/renderengine/threaded/RenderEngineThreaded.cpp
index b41e843..8aa41b3 100644
--- a/libs/renderengine/threaded/RenderEngineThreaded.cpp
+++ b/libs/renderengine/threaded/RenderEngineThreaded.cpp
@@ -90,7 +90,6 @@
     }
 
     mRenderEngine = factory();
-    mIsProtected = mRenderEngine->isProtected();
 
     pthread_setname_np(pthread_self(), mThreadName);
 
@@ -255,41 +254,11 @@
     return mRenderEngine->getMaxViewportDims();
 }
 
-bool RenderEngineThreaded::isProtected() const {
-    waitUntilInitialized();
-    std::lock_guard lock(mThreadMutex);
-    return mIsProtected;
-}
-
 bool RenderEngineThreaded::supportsProtectedContent() const {
     waitUntilInitialized();
     return mRenderEngine->supportsProtectedContent();
 }
 
-void RenderEngineThreaded::useProtectedContext(bool useProtectedContext) {
-    if (isProtected() == useProtectedContext ||
-        (useProtectedContext && !supportsProtectedContent())) {
-        return;
-    }
-
-    {
-        std::lock_guard lock(mThreadMutex);
-        mFunctionCalls.push([useProtectedContext, this](renderengine::RenderEngine& instance) {
-            ATRACE_NAME("REThreaded::useProtectedContext");
-            instance.useProtectedContext(useProtectedContext);
-            if (instance.isProtected() != useProtectedContext) {
-                ALOGE("Failed to switch RenderEngine context.");
-                // reset the cached mIsProtected value to a good state, but this does not
-                // prevent other callers of this method and isProtected from reading the
-                // invalid cached value.
-                mIsProtected = instance.isProtected();
-            }
-        });
-        mIsProtected = useProtectedContext;
-    }
-    mCondition.notify_one();
-}
-
 void RenderEngineThreaded::cleanupPostRender() {
     if (canSkipPostRenderCleanup()) {
         return;
@@ -334,6 +303,7 @@
         mFunctionCalls.push([resultPromise, display, layers, buffer, useFramebufferCache,
                              fd](renderengine::RenderEngine& instance) {
             ATRACE_NAME("REThreaded::drawLayers");
+            instance.updateProtectedContext(layers, buffer);
             instance.drawLayersInternal(std::move(resultPromise), display, layers, buffer,
                                         useFramebufferCache, base::unique_fd(fd));
         });