A parametrized Config singleton

Here is an attempt to code a specific kind of Singleton - the one geared for our configuration needs. It needs to be initialized with a configuration location, do not allow copies or other instances, and provide feedback or fail in case of problems. Prefer graceful feedback from violated workflows. While not performance-critical (especially initialization), the correct behavior and thread safety are needed.

Configuration location differs in production/testing/development scenarios - or may be acquired via a network broadcast in the future. Getting location is outside of this class scope.

So, here is the Config.h header:

class Config {

public:
  typedef std::function DupInitCB;

  static void init(std::string loc, DupInitCB err_cb);
  static Config& instance();

private:
  static std::unique_ptr cfg;
  static std::mutex init_mutex;

  std::string url;

  Config(std::string loc);
  Config(const Config& src);              // not implemented
  Config& operator=(const Config& right); // not implemented

public:
  std::string val(std::string key);
};

The implementation Config.cpp:

#include "Config.h"

std::unique_ptr Config::cfg;
std::mutex Config::init_mutex;

void Config::init(std::string loc, DupInitCB err_cb) {
  std::lock_guard lock(init_mutex);
  if (cfg != nullptr) {
    err_cb(loc);
  } else {
    cfg.reset(new Config(loc));
  };
};

Config& Config::instance() {
  if (cfg == nullptr) throw std::logic_error("Requested uninitialized Config");
  return *cfg.get();
}

Config::Config(std::string loc) : url(loc) {};

std::string Config::val(std::string key) {
  return key + " key requested at the location " + url;
}

Finally, a usage example:

```
#include "Config.h"

int _tmain(int argc, _TCHAR* argv[])
{

try {
std::cout << Config::instance().val("Upstream") << std::endl;
std::cerr << "Did not trow an exception when called an uninitialized Config." << std::endl;
return 1;
}
catch (std::logic_error e) {
std::cout << "Prop

Does the instance() method need to be synchronized?

Theoretically, yes: because unique_ptr isn't thread-safe: therefore you should not call get while another thread is calling reset.

It may be safe in practice.

Alternatively you don't need all the functionality of a unique_ptr.
Instead you could implement its functionality using a dumb pointer, which you can reason about and/or access using the STL atomic functions.


Is there a better way to handle violated workflow from the instance() method than throwing an exception?

You could return a pointer instead of a reference, and return a null pointer instead of throwing an exception: and expect the caller to check for null before dereferencing the pointer.

Another possibility may be the Null Object pattern: if there's no Config instance then return a reference to an empty Config instance, or a Config instance initialized with suitable default values.


What are the dangers of call-back from under the lock?

In general the danger of callback from under a lock is 'deadly embrace' a.k.a. 'deadlock'; for example:

Thread 1:

// get the lock
lock_guard lock(foo_mutex);
// get the config
config.init("foo", [](string loc){ cout  lock(foo_mutex);
    cout << "I don't care";
});

Imagine the following sequence:

• Thread 2 enters config and acquires config's lock

• Thread 1 acquires the the foo_mutex

• Thread 1 blocks on the config mutex (owned by thread 2)

• Thread 2 enters the callback

• Thread 2 callback blocks on the foo_mutex (owned by thread 1)

It's safe to acquire multiple locks if you always acquire them in the same sequence.
With a callback it's difficult for the author of the Config class to predict what locks might be held before and/or during the callback.
This is a hidden problem for low-level library classes: for example if Logger uses Config in its implementation, Logger and Config each have their own mutex, and a callback from Config tries to invoke a Logger method.

Another problem with callback from under a lock is that you don't know how long the callback is going to take. If the callback takes a long time to execute, then the lock will be held for a long time.


OK, so callback is now gone.

Perhaps you can keep the callback provided you unlock before calling it:

void Config::init(std::string loc, DupInitCB err_cb) {
  bool succeeded;

  { // scope of lock_guard
    std::lock_guard lock(init_mutex);
    if (cfg != nullptr) {
      succeeded = false;
    } else {
      cfg.reset(new Config(loc));
      succeeded = true;
    };
  } // lock_guard is released here

  // error callback after releasing the lock`enter code here`
  if (!succeeded)
    err_cb(loc);
};

Or the code within the artificial { ... } scope above could be a private method which returns bool, called from init, and named something like locked_init.

StackExchange Code Review Q#40205, answer score: 5