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

Commit c626cebf authored by Dominik Laskowski's avatar Dominik Laskowski Committed by Android (Google) Code Review
Browse files

Merge changes from topics "presubmit-am-54728ad3f0014d59be7b146d10223f7f",... 

Merge changes from topics "presubmit-am-54728ad3f0014d59be7b146d10223f7f", "presubmit-am-effdf607213e46929e9d2fba03a41994" into tm-dev

* changes:
  SF: Generalize thread safety guards
  FTL: Add thread safety helpers
parents ac7b96d7 298b08e5

include/ftl/fake_guard.h

0 → 100644
+90 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#pragma once



#define FTL_ATTRIBUTE(a) __attribute__((a))



namespace android::ftl {



// Granular alternative to [[clang::no_thread_safety_analysis]]. Given a std::mutex-like object,

// FakeGuard suppresses enforcement of thread-safe access to guarded variables within its scope.

// While FakeGuard is scoped to a block, there are macro shorthands for a single expression, as

// well as function/lambda scope (though calls must be indirect, e.g. virtual or std::function):

//

//   struct {

//     std::mutex mutex;

//     int x FTL_ATTRIBUTE(guarded_by(mutex)) = -1;

//

//     int f() {

//       {

//         ftl::FakeGuard guard(mutex);

//         x = 0;

//       }

//

//       return FTL_FAKE_GUARD(mutex, x + 1);

//     }

//

//      std::function<int()> g() const {

//        return [this]() FTL_FAKE_GUARD(mutex) { return x; };

//      }

//   } s;

//

//   assert(s.f() == 1);

//   assert(s.g()() == 0);

//

// An example of a situation where FakeGuard helps is a mutex that guards writes on Thread 1, and

// reads on Thread 2. Reads on Thread 1, which is the only writer, need not be under lock, so can

// use FakeGuard to appease the thread safety analyzer. Another example is enforcing and documenting

// exclusive access by a single thread. This is done by defining a global constant that represents a

// thread context, and annotating guarded variables as if it were a mutex (though without any effect

// at run time):

//

//   constexpr class [[clang::capability("mutex")]] {

//   } kMainThreadContext;

//

template <typename Mutex>

struct [[clang::scoped_lockable]] FakeGuard final {

  explicit FakeGuard(const Mutex& mutex) FTL_ATTRIBUTE(acquire_capability(mutex)) {}

  [[clang::release_capability()]] ~FakeGuard() {}



  FakeGuard(const FakeGuard&) = delete;

  FakeGuard& operator=(const FakeGuard&) = delete;

};



}  // namespace android::ftl



// TODO: Enable in C++23 once standard attributes can be used on lambdas.

#if 0

#define FTL_FAKE_GUARD1(mutex) [[using clang: acquire_capability(mutex), release_capability(mutex)]]

#else

#define FTL_FAKE_GUARD1(mutex)             \

  FTL_ATTRIBUTE(acquire_capability(mutex)) \

  FTL_ATTRIBUTE(release_capability(mutex))

#endif



// The parentheses around `expr` are needed to deduce an lvalue or rvalue reference.

#define FTL_FAKE_GUARD2(mutex, expr)            \

  [&]() -> decltype(auto) {                     \

    const android::ftl::FakeGuard guard(mutex); \

    return (expr);                              \

  }()



#define FTL_MAKE_FAKE_GUARD(arg1, arg2, guard, ...) guard



// The void argument suppresses a warning about zero variadic macro arguments.

#define FTL_FAKE_GUARD(...) \

  FTL_MAKE_FAKE_GUARD(__VA_ARGS__, FTL_FAKE_GUARD2, FTL_FAKE_GUARD1, void)(__VA_ARGS__)

libs/ftl/Android.bp

+2 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ cc_test { 
        "cast_test.cpp",

        "concat_test.cpp",

        "enum_test.cpp",

        "fake_guard_test.cpp",

        "future_test.cpp",

        "small_map_test.cpp",

        "small_vector_test.cpp",
@@ -29,6 +30,7 @@ cc_test { 
        "-Werror",

        "-Wextra",

        "-Wpedantic",

        "-Wthread-safety",

    ],



    header_libs: [

libs/ftl/fake_guard_test.cpp

0 → 100644
+49 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



#include <ftl/fake_guard.h>

#include <gtest/gtest.h>



#include <functional>

#include <mutex>



namespace android::test {



// Keep in sync with example usage in header file.

TEST(FakeGuard, Example) {

  struct {

    std::mutex mutex;

    int x FTL_ATTRIBUTE(guarded_by(mutex)) = -1;



    int f() {

      {

        ftl::FakeGuard guard(mutex);

        x = 0;

      }



      return FTL_FAKE_GUARD(mutex, x + 1);

    }



    std::function<int()> g() const {

      return [this]() FTL_FAKE_GUARD(mutex) { return x; };

    }

  } s;



  EXPECT_EQ(s.f(), 1);

  EXPECT_EQ(s.g()(), 0);

}



}  // namespace android::test

services/surfaceflinger/DisplayDevice.h

+11 −13
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include <string>

#include <unordered_map>



#include <android-base/thread_annotations.h>

#include <android/native_window.h>

#include <binder/IBinder.h>

#include <gui/LayerState.h>
@@ -28,6 +29,7 @@ 
#include <renderengine/RenderEngine.h>

#include <system/window.h>

#include <ui/DisplayId.h>

#include <ui/DisplayIdentification.h>

#include <ui/DisplayState.h>

#include <ui/GraphicTypes.h>

#include <ui/HdrCapabilities.h>
@@ -39,15 +41,11 @@ 
#include <utils/RefBase.h>

#include <utils/Timers.h>



#include "MainThreadGuard.h"



#include <ui/DisplayIdentification.h>

#include "DisplayHardware/DisplayMode.h"

#include "DisplayHardware/Hal.h"

#include "DisplayHardware/PowerAdvisor.h"



#include "Scheduler/RefreshRateConfigs.h"



#include "ThreadContext.h"

#include "TracedOrdinal.h"



namespace android {
@@ -99,9 +97,9 @@ public: 
    void setLayerStack(ui::LayerStack);

    void setDisplaySize(int width, int height);

    void setProjection(ui::Rotation orientation, Rect viewport, Rect frame);

    void stageBrightness(float brightness) REQUIRES(SF_MAIN_THREAD);

    void persistBrightness(bool needsComposite) REQUIRES(SF_MAIN_THREAD);

    bool isBrightnessStale() const REQUIRES(SF_MAIN_THREAD);

    void stageBrightness(float brightness) REQUIRES(kMainThreadContext);

    void persistBrightness(bool needsComposite) REQUIRES(kMainThreadContext);

    bool isBrightnessStale() const REQUIRES(kMainThreadContext);

    void setFlags(uint32_t flags);



    ui::Rotation getPhysicalOrientation() const { return mPhysicalOrientation; }
@@ -109,7 +107,7 @@ public: 


    static ui::Transform::RotationFlags getPrimaryDisplayRotationFlags();



    std::optional<float> getStagedBrightness() const REQUIRES(SF_MAIN_THREAD);

    std::optional<float> getStagedBrightness() const REQUIRES(kMainThreadContext);

    ui::Transform::RotationFlags getTransformHint() const;

    const ui::Transform& getTransform() const;

    const Rect& getLayerStackSpaceRect() const;
@@ -209,15 +207,15 @@ public: 
    bool setDesiredActiveMode(const ActiveModeInfo&) EXCLUDES(mActiveModeLock);

    std::optional<ActiveModeInfo> getDesiredActiveMode() const EXCLUDES(mActiveModeLock);

    void clearDesiredActiveModeState() EXCLUDES(mActiveModeLock);

    ActiveModeInfo getUpcomingActiveMode() const REQUIRES(SF_MAIN_THREAD) {

    ActiveModeInfo getUpcomingActiveMode() const REQUIRES(kMainThreadContext) {

        return mUpcomingActiveMode;

    }



    void setActiveMode(DisplayModeId) REQUIRES(SF_MAIN_THREAD);

    void setActiveMode(DisplayModeId) REQUIRES(kMainThreadContext);

    status_t initiateModeChange(const ActiveModeInfo&,

                                const hal::VsyncPeriodChangeConstraints& constraints,

                                hal::VsyncPeriodChangeTimeline* outTimeline)

            REQUIRES(SF_MAIN_THREAD);

            REQUIRES(kMainThreadContext);



    // Return the immutable list of supported display modes. The HWC may report different modes

    // after a hotplug reconnect event, in which case the DisplayDevice object will be recreated.
@@ -304,7 +302,7 @@ private: 
    ActiveModeInfo mDesiredActiveMode GUARDED_BY(mActiveModeLock);

    TracedOrdinal<bool> mDesiredActiveModeChanged

            GUARDED_BY(mActiveModeLock) = {"DesiredActiveModeChanged", false};

    ActiveModeInfo mUpcomingActiveMode GUARDED_BY(SF_MAIN_THREAD);

    ActiveModeInfo mUpcomingActiveMode GUARDED_BY(kMainThreadContext);

};



struct DisplayDeviceState {

services/surfaceflinger/Layer.cpp

+4 −3
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ 
#include <cutils/native_handle.h>

#include <cutils/properties.h>

#include <ftl/enum.h>

#include <ftl/fake_guard.h>

#include <gui/BufferItem.h>

#include <gui/LayerDebugInfo.h>

#include <gui/Surface.h>
@@ -2045,10 +2046,10 @@ LayerProto* Layer::writeToProto(LayersProto& layersProto, uint32_t traceFlags) { 
    writeToProtoCommonState(layerProto, LayerVector::StateSet::Drawing, traceFlags);



    if (traceFlags & LayerTracing::TRACE_COMPOSITION) {

        ftl::FakeGuard guard(mFlinger->mStateLock); // Called from the main thread.



        // Only populate for the primary display.

        UnnecessaryLock assumeLocked(mFlinger->mStateLock); // called from the main thread.

        const auto display = mFlinger->getDefaultDisplayDeviceLocked();

        if (display) {

        if (const auto display = mFlinger->getDefaultDisplayDeviceLocked()) {

            const auto compositionType = getCompositionType(*display);

            layerProto->set_hwc_composition_type(static_cast<HwcCompositionType>(compositionType));

            LayerProtoHelper::writeToProto(getVisibleRegion(display.get()),