1

I am creating a system state class that needs be thread safe in reading/writing. I am implementing it using the Singleton Design Pattern. I have based it off of pointers from these Stackoverflow questions:

  1. Singleton: How it Should be Used
  2. C++ Singleton Design Pattern

Here is a condensed example of my code. Essentially it is just some state variables I need to keep track of and share across several threads. I know for a fact there will only ever needs be one state instance and I know that it needs to thread safe and constant. Will this implementation ensure that?

// System_State.h
class System_State
{
public:
    static System_State &getInstance()
    {
        static System_State stateInstance;
        return stateInstance;
    }

    System_State(System_State const&)   = delete;
    void operator=(System_State const&) = delete;

    // Example Setter with mutex guard
    void setWifiConnectedStatus(uint8_t _status)
    {
        std::lock_guard<std::mutex> lock(mtx);
        Wifi.ConnectedStatus = _status;
    }

private:
    System_State() 
    {
        initializeState();
    }

    void initializeState();

    std::mutex mtx;

    static struct WifiService
    {
        uint8_t     ConnectedStatus;
        std::string CurrentConnection;
        std::string SavedNetworks;
        std::string AvailableNetworks;
        std::string ForgetNetwork;
        std::string ConnectTo;
        std::string DisconnectFrom;
    } Wifi;

    static struct ScanService
    {
        std::string PerformScan;
        std::string ScanStatus;
        uint8_t     ScanProgress;
    } Scan;

    static struct UploadDataService
    {
        std::string PerformUpload;
        uint8_t     UploadProgress;
        uint8_t     ItemsWaitingToBeUploaded;
    } UploadData;

    static std::string LEDControlBrightness;
    static uint8_t     BatteryPercentage;
};

And here is some example main

//main.cpp
#include <thread>

void loop1()
{
    System_State state = System_State::getInstance();
    while(true)
    {
        //do randomness with state
    }
}

void loop2()
{
    System_State state2 = System_State::getInstance();
    while(true)
    {
        //do randomness with state2
    }
}

int main()
{
    std::thread t1(loop1);
    std::thread t2(loop2);
    // do and join and all that!
    return 0;
}
SergeyA
  • 61,605
  • 5
  • 78
  • 137
Patrick Walter
  • 73
  • 8

2 Answers2

1

Yours is not idiomatic singleton, because it is still prone to so-called "static initialization order fiasco".

An idiomatic singleton doesn't have a static class member, instead it's instance function looks like

static MySingleton& instance() {
    static MySingleton the_ton;
    return the_ton;
}

More on the fiasco: static initialization order fiasco

SergeyA
  • 61,605
  • 5
  • 78
  • 137
0

No, this may not work in a more complex case, but not for thread-safety reason, but because of the undefined behavior of static object initialization.

Let's say that your object here:

static System_State stateInstance;

requires another static object that you retrieve with another static getInstance. You have no assurance that this will return a valid/constructed object.

If you can ensure that this object doesn't require another static object, then you should be fine. But this is the issue with lots of singleton constructions, you have to balance when it is constructed and thread-safety.

Matthieu Brucher
  • 21,634
  • 7
  • 38
  • 62
  • *"this is the issue with every singleton construction"* - not every singleton construction. The "mayer singleton" solves the static initialization problem. Interestingly the OP links to it but doesn't use it: https://stackoverflow.com/a/1008289/3807729 – Galik Mar 04 '19 at 16:29
  • Oh yes indeed, in C++11 there is a stronger guarantee on these, thanks for the reminder. – Matthieu Brucher Mar 04 '19 at 16:32
  • I should have put in the question that I am using c++11. I will update that as well – Patrick Walter Mar 04 '19 at 16:34
  • In that case, just use the link @Galik posted and use this simple singleton. – Matthieu Brucher Mar 04 '19 at 16:37