My singleton can be called multiple times

Question

I have implemented an singleton based on c++ 11. However the constructor can be called multiple times in some cases.

The class will be compiled to static lib and used by other so lib (more than one so lib). And the system is a multi-thread system (run in the Android HAL level)

/// The .h file:

class Logger
{
public:

    /// Return the singleton instance of Logger
    static Logger& GetInstance() {
        static Logger s_loggerSingleton;
        return s_loggerSingleton;
    }

private:

    /// Constructor
    Logger();
    /// Destructor
    ~Logger();
}

/// the .cpp file

Logger::Logger()
{
   ALOGE("OfflineLogger create");
}

Logger::~Logger()
{

}

It should be created once eg:

03-21 01:52:20.785   728  4522 E         : OfflineLogger create

However I can see it has been created more than once

03-21 01:52:20.785   728  4522 E         : OfflineLogger create
03-21 01:52:20.863   728  2274 E         : OfflineLogger create
03-21 01:52:20.977   728  2273 E         : OfflineLogger create
03-21 01:52:26.370   728  4522 E         : OfflineLogger create

Questions:

1. Anything wrong with my singleton design? Is it a thread-safe issue?

2. Seems like my singleton works fine in one so scope, but each so lib which includes my singleton will create its own singleton, so that my singleton is no longer “be a singleton”. Is the problem caused from each dynamic linking to new so and the "static variable" becomes "local static"? Is it possible? If so, how to fix?

Answer 1

1. Anything wrong with my singleton design? Is it a thread-safe issue?

No. Initialization of function local static variables is guaranteed to be thread-safe by the standard.

2. Seems like my singleton works fine in one so scope, but each so lib which include my singleton will create its own singleton, so that my singleton is no longer “be a singleton”. Is the problem caused from each dynamic linking to new so and the "staic veriable" become "local static"? Is it possible? If so, how to fix

That is the correct conclusion.

Instead of creating a static library that contains the implementation of the singleton, make it a dynamic library.

Answer 2

Singletons are hard, especially with shared libraries.

Each of your shared libraries has an independent copy of the non-shared library. Without extra care, each will have a copy of the singleton.

In order to have non-trivial singletons, what I have had to do was

1. Create an extremely low level library to help with singletons -- call it LibSingleton

2. Create a singleton template that knows the type of the singleton. It uses magic statics to send a request to the LibSingleton with a size, typeid(T).name() key, and type-erased construction and destruction code. LibSingleton returns a reference counting RAII object.

3. LibSingleton uses a shared mutex to either return a previously constructed object that matches the name/size or constructs it. If it constructs the object, it stores the destruction code.

4. When the last reference-counted handle to the LibSingleton data goes away, LibSingleton runs the destruction code and cleans up the memory in its unordered map.

This permits really simple singletons to be used nearly anywhere.

template<class T>
class singleton {
public:
  static T& Instance() {
    static auto smart_ptr = LibSingleton::RequestInstance(
      typeid(T).name(),
      sizeof(T),
      [](void* ptr){ return ::new( ptr ) T{}; },
      [](void* ptr){ static_cast<T*>(ptr)->~T(); }
    );
    if (!smart_ptr)
      exit(-1); // or throw something
    return *static_cast<T*>(smart_ptr.get());
  }
protected:
  singleton() = default;
  ~singleton() = default;
private:
  singleton(singleton&&) = delete;
  singleton& operator=(singleton&&) = delete;
};

use looks like:

struct Logger : LibSingleton::singleton<Logger> {
  friend class LibSingleton::singleton<Logger>;
  void do_log( char const* sting ) {}
private:
  Logger() { /* ... */ }
};

Answer 3

Here's an idea:Rather than use a singleton,which is both a hard problem in your environment and known to be problematic for testing and maintenance,just design your code to only create one of the object in question.

Answer 4

static variable should be moved to .cpp file.

Simple way is to keep only declaration of getInstance() in .h and move implementation to .cpp file.

Answer 5

It may be that your header file is being defined multiple times (it's the case if multiple files are including this header file. Try adding a guard around the header file to prevent it from being re-defined if it has already been defined once.

Depending on your C++ compiler, you could actually just add #pragma once as the first line in your file, like this

#pragma once
class Logger
{
    public:

    /// Return the singleton instance of Logger
    static Logger& GetInstance() {
    static Logger s_loggerSingleton;
    return s_loggerSingleton;
}

private:

    /// Constructor
    Logger();
    /// Destructor
    ~Logger();
}

The intended effect is that of the most general alternative, which is to add a macro definition like this

#ifndef LOGGER_H
#define LOGGER_H
class Logger
{
    public:

    /// Return the singleton instance of Logger
    static Logger& GetInstance() {
    static Logger s_loggerSingleton;
    return s_loggerSingleton;
}

private:

    /// Constructor
    Logger();
    /// Destructor
    ~Logger();
}
#endif LOGGER_H