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

Commit e6abd70b authored by Gil Dekel's avatar Gil Dekel
Browse files

FlagManager: Move mInstance to getMutableInstance 

The static globals mOnce and mInstance in FlagManager could potentially
cause initialization issues between objects when referencing each other.

We solve this by:
1. Move mInstance to getMutableInstance() in FlagManager, and
2. Get rid of mOnce in favor of the static initialization of mInstance
   from within getMutableInstance().

We also remove ConstructorTag{} in favor of a default private
constructor, since a specialized ctor for std::make_unique is no longer
necessary.

Flag: EXEMPT refactor
Bug: 421221131
Test: libsurfaceflinger_unittest
Change-Id: I22e838a8e25a266f155527121e68f89571ba03fb
parent 2f4e35ae

services/surfaceflinger/common/FlagManager.cpp

+2 −11
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ 
#include <log/log.h>

#include <renderengine/RenderEngine.h>

#include <server_configurable_flags/get_flags.h>

#include <cinttypes>



#include <android_companion_virtualdevice_flags.h>

#include <android_hardware_flags.h>
@@ -38,10 +37,6 @@ using namespace com::android::graphics::surfaceflinger; 


static constexpr const char* kExperimentNamespace = "surface_flinger_native_boot";



std::unique_ptr<FlagManager> FlagManager::mInstance;

std::once_flag FlagManager::mOnce;



FlagManager::FlagManager(ConstructorTag) {}

FlagManager::~FlagManager() = default;



namespace {
@@ -72,12 +67,8 @@ const FlagManager& FlagManager::getInstance() { 
}



FlagManager& FlagManager::getMutableInstance() {

    std::call_once(mOnce, [&] {

        LOG_ALWAYS_FATAL_IF(mInstance, "Instance already created");

        mInstance = std::make_unique<FlagManager>(ConstructorTag{});

    });



    return *mInstance;

    static FlagManager sInstance;

    return sInstance;

}



void FlagManager::markBootCompleted() {

services/surfaceflinger/common/include/common/FlagManager.h

+1 −10
Original line number Diff line number Diff line
@@ -17,9 +17,7 @@ 
#pragma once



#include <atomic>

#include <cstdint>

#include <functional>

#include <mutex>

#include <optional>

#include <string>
@@ -27,15 +25,10 @@ namespace android { 
// Manages flags for SurfaceFlinger, including default values, system properties, and Mendel

// experiment configuration values. Can be called from any thread.

class FlagManager {

private:

    // Effectively making the constructor private, while allowing std::make_unique to work

    struct ConstructorTag {};



public:

    static const FlagManager& getInstance();

    static FlagManager& getMutableInstance();



    FlagManager(ConstructorTag);

    virtual ~FlagManager();



    void markBootCompleted();
@@ -125,6 +118,7 @@ protected: 
private:

    friend class TestableFlagManager;



    FlagManager() = default;

    FlagManager(const FlagManager&) = delete;



    void dumpFlag(std::string& result, bool readonly, const char* name,
@@ -132,8 +126,5 @@ private: 


    std::atomic_bool mBootCompleted = false;

    std::atomic_bool mUnitTestMode = false;



    static std::unique_ptr<FlagManager> mInstance;

    static std::once_flag mOnce;

};

} // namespace android

services/surfaceflinger/tests/unittests/FlagManagerTest.cpp

+1 −1
Original line number Diff line number Diff line
@@ -33,7 +33,7 @@ using testing::Return; 


class TestableFlagManager : public FlagManager {

public:

    TestableFlagManager() : FlagManager(ConstructorTag{}) { markBootCompleted(); }

    TestableFlagManager() { markBootCompleted(); }

    ~TestableFlagManager() = default;



    MOCK_METHOD(std::optional<bool>, getBoolProperty, (const char*), (const, override));