Singleton design pattern study

Question

I am learning design patterns and the singleton is one of them. What are your opinions on the code? Please feel free to share.

I know singletons are not popular because of complications in testing. I tested the program by running it repeatedly and checking the results manually.

During my study of singletons I found that, if the constructor of the singleton is slow, multiple instances of the singleton are created even when declared static in a method. One thread can be busy constructing while the next thread starts starts constructing, even when the variable is declared static in a function block. To me this was new and baffling. So I had to start using mutexes for thread safety. The goal of the CSleeper class is to have the threads catch up with each other.

Compiling:

g++ -I inc -Wall -std=gnu++11 -pthread -c CPrinter.cpp -o dbg/bin/CPrinter.o
g++ -I inc -Wall -std=gnu++11 -pthread -c CSingleton.cpp -o dbg/bin/CSingleton.o
g++ -I inc -Wall -std=gnu++11 -pthread -c CSleeper.cpp -o dbg/bin/CSleeper.o
g++ -I inc -Wall -std=gnu++11 -pthread -c main.cpp -o dbg/bin/main.o
g++ -g -I inc -Wall -std=gnu++11 -pthread -lm -lpthread dbg/bin/CPrinter.o dbg/bin/CSingleton.o dbg/bin/CSleeper.o dbg/bin/main.o -o Singleton_dbg.exe

CPrinter.cpp

#include <mutex>
#include <iostream>
#include "CPrinter.h"
static std::mutex gMutex;
void CPrinter::Printer(std::stringstream& aStringStream)
{
 std::lock_guard<std::mutex> lock(gMutex);
 std::cout << aStringStream.str().c_str();
 aStringStream.str("");
}
void CPrinter::Printer(std::string& aString)
{
 std::stringstream ss;
 ss << aString << std::endl;
 Printer(ss);
}

CPrinter.h

#ifndef PRINTER_H
#define PRINTER_H
#include <sstream>
struct CPrinter
{
 static void Printer(std::stringstream& aStringStream);
 static void Printer(std::string& aString);
};
#endif // PRINTER_H

CSingleton.cpp

#include <mutex>
#include <string>
#include "CSleeper.h"
#include "CSingleton.h"
#include "CPrinter.h"
static std::mutex gStdMutexPublic; // Entry level to the singleton
static std::mutex gStdMutexConstruct; // Around the construction if the singleton.
//////////////////////////////////////////////////////
CSingleton::CSingleton() : iTestMember("empty")
{
 std::stringstream stringStream;
 stringStream << __PRETTY_FUNCTION__ << " Started." << std::endl;
 CPrinter::Printer(stringStream);
 CSleeper::Sleep() ;
 stringStream << __PRETTY_FUNCTION__ << " Finished." << std::endl;
 CPrinter::Printer(stringStream);
}
//////////////////////////////////////////////////////
CSingleton& CSingleton::GetSingleton()
{
 std::lock_guard<std::mutex> lock(gStdMutexConstruct);
 CSingleton& mySingletonRef = Construct();
 CSleeper::Sleep();
 return mySingletonRef;
}
//////////////////////////////////////////////////////
CSingleton& CSingleton::Construct()
{
 // This method should be protected by a mutex outside of the method.
 static CSingleton myCSingleton;
 std::stringstream stringStream;
 stringStream << __PRETTY_FUNCTION__ << ": &myCSingleton= " << &myCSingleton << std::endl;
 CPrinter::Printer(stringStream);
 CSleeper::Sleep();
 return myCSingleton;
}
//////////////////////////////////////////////////////
std::string CSingleton::GetCopyOfTestMember() const
{
 std::lock_guard<std::mutex> lock(gStdMutexPublic);
 std::string r;
 for (size_t index = 0; index < iTestMember.length(); index++)
 {
 CSleeper::Sleep();
 // SLow copy for checking of thread safety
 r += iTestMember[index];
 CPrinter::Printer(r);
 }
 return r;
}
//////////////////////////////////////////////////////
void CSingleton::SetTestMember(const std::string& aTestMember)
{
 std::lock_guard<std::mutex> lock(gStdMutexPublic);
 std::stringstream stringStream;
 stringStream << __PRETTY_FUNCTION__ << ": changing from " << iTestMember << " to " << aTestMember << "." << std::endl;
 CPrinter::Printer(stringStream);
 iTestMember = "";
 for (size_t index = 0; index < aTestMember.length(); index++)
 {
 CSleeper::Sleep();
 // SLow copy for checking of thread safety
 iTestMember += aTestMember[index];
 CPrinter::Printer(iTestMember);
 }
}

CSingleton.h

#ifndef CSINGLETON_H
#define CSINGLETON_H
#include <string>
// This class should be the ultimate, thread-safe, failure proof
// singleton implementation in C++.
class CSingleton
{
public:
 std::string GetCopyOfTestMember() const;
 void SetTestMember(const std::string& aTestMember);
 static CSingleton& GetSingleton();
private:
 CSingleton();
 static CSingleton& Construct();
 ~CSingleton() = default; // dtor
 CSingleton(const CSingleton&) = delete; // copy ctor
 CSingleton(CSingleton&&) = delete; // move ctor
 CSingleton& operator=(CSingleton&) = default; // copy assignment op
 CSingleton& operator=(CSingleton&&) = delete; // move assignment op
 std::string iTestMember;
};
#endif // CSINGLETON_H

CSleeper.cpp

#include <unistd.h>
#include <stdlib.h>
#include <ctime>
#include "CSleeper.h"
static unsigned int g_seed = static_cast<unsigned int>(std::time(nullptr) % 0xFFFFFFFF);
void CSleeper::Sleep()
{
 rand_r(&g_seed);
 usleep(g_seed % 1000);
}

CSleeper.h

#ifndef CSLEEPER_H
#define CSLEEPER_H
struct CSleeper
{
 static void Sleep();
};
#endif // CSLEEPER_H

main.cpp

#include <thread> // std::thread
#include <sstream>
#include <string>
#include "CSleeper.h"
#include "CSingleton.h"
#include "CPrinter.h"
void SingletonTester(const int aThreadId, const std::string& aSetMember)
{
 std::stringstream ss;
 ss << __PRETTY_FUNCTION__ << ": Thread " << aThreadId << " started. " << std::endl;
 CPrinter::Printer(ss);
 CSleeper::Sleep();
 CSingleton& mySingleton = CSingleton::GetSingleton();
 CSleeper::Sleep();
 ss << __PRETTY_FUNCTION__ << ": Thread " << aThreadId << " is going to set singleton member to " << aSetMember << "." << std::endl;
 CPrinter::Printer(ss);
 mySingleton.SetTestMember(aSetMember);
 CSleeper::Sleep();
 ss << __PRETTY_FUNCTION__ << ": Thread " << aThreadId << " has received singleton member value " << mySingleton.GetCopyOfTestMember() << "." << std::endl;
 CPrinter::Printer(ss);
 CSleeper::Sleep();
 ss << __PRETTY_FUNCTION__ << ": Finished." << std::endl;
 CPrinter::Printer(ss);
}
int main()
{
 std::stringstream ss;
 std::string thread1string = "twenty";
 std::string thread2string = "thirty";
 ss << "Singleton tester started." << std::endl;
 CPrinter::Printer(ss);
 std::thread threadOne(SingletonTester, 20, thread1string);
 std::thread threadTwo(SingletonTester, 30, thread2string);
 threadOne.join();
 threadTwo.join();
 ss << "Singleton tester finished." << std::endl;
 CPrinter::Printer(ss);
}

• Artemy Vysotsky: I'd think Printer API should be such that user did not need the stringstream: CPrinter::Printer() << "Singleton tester started." << std::endl;. Of course it will require to have thread local storage to collect the line before printing. Another thing - would be nice to have a possibility to replace cout with ofstream to file
• TradingDerivatives.eu: @ArtemyVysotsky: The purpose of the code is about a singleton implementation. It is not about printing. The CPrinter class is just a thread-safe printing facility. Dumping the output of the program to file does not add to the singleton design pattern in my view.
• user7860670: "singletons are not popular" they are very popular, most of the programs contain singletons in implicit or explicit form. "complications in testing" is not true either. You may have issues with testing your implementation because it is using so called "Meyers Singleton" storing singleton instance in local static variable and therefore giving up control over singleton lifetime. Personally I call "Meyers Singleton" an anti-pattern.
• TradingDerivatives.eu: @VTT: Thanks for your help. In principle it is possible to implement a singleton differently. The software designer could decide to implement a function to destroy the singleton instance. With that a recreating call is also needed. This scheme allows only zero or one instances of the object and the singleton must check its allocation before use. The possibility to destroy the instance can help testing because constructing it under different circumstances can be tested. I am not sure that this is still called a singleton, but semantics aside, is this what you mean? Can you elaborate? Thanks.
• user7860670: There is a thing called "single responsibility principle". If you think about your singleton then you can notice that it actually has at least 3 responsibilities: 1) obviously provides a method to get a class instance for common use; 2) controls lifetime of this common class instance; 3) controls how many instances of this class there can exists. Each of them can be implemented and used completely separately. #3 - class instances counter (not forced to a single instance btw) #2 - automatic storage duration or smart pointers #1 - this one looks like a job for a singleton.

Answer 1

First note. Since C++11 the construction of static members has ben thread safe. So there is no need for locking.

There should only be one singelton. So all copying should be disabled.

// Having copy assignment enabled seems to go against his.
CSingleton& operator=(CSingleton&) = default; // copy assignment op

I know singletons are not popular because of complications in testing. I tested the program by running it repeatedly and checking the results manually.

Globally mutable state is not good practice. Because of tight coupling and side affect driven code (code that has side affects outside of the parameters is difficult to handle in real life as well as for testing).

During my study of singletons I found that, if the constructor of the singleton is slow, multiple instances of the singleton are created even when declared static in a method.

The you must be using a pre C++11 compiler or a compiler that has a bug (or the bug in your code I pointed to above is causing you problems).

One thread can be busy constructing while the next thread starts starts constructing, even when the variable is declared static in a function block.

Check the version of your compiler. That should not happen in C++11 compliant compiler. So I suspect that this is something you are doing wrong.

• TradingDerivatives.eu: Indeed, when I delete the assignment operator the code compiles still. I will check it out.