6
\$\begingroup\$

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<void(std::string)> DupInitCB;

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

private:
  static std::unique_ptr<Config> 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> Config::cfg;
std::mutex Config::init_mutex;

void Config::init(std::string loc, DupInitCB err_cb) {
  std::lock_guard<std::mutex> 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 << "Properly thrown an error when called an uninitialized Config." << std::endl;
  }
  catch (...) {
    std::cerr << "Thrown an unexpected exception when called an uninitialized Config." << std::endl;
    return 1;
  }

  Config::init(
    "http://cnf.local/dev",
    [](std::string loc){ std::cout << "Duplicate Config init (" << loc << ")" << std::endl; }
  );

  Config::init(
    "http://cnf.local/prod",
    [](std::string loc){ std::cout << "Duplicate Config init (" << loc << ")" << std::endl; }
  );

  std::cout << Config::instance().val("Upstream") << std::endl;
  return 0;
}

A buildable VC++ 2012 solution is posted at the GitHub repo.

The specific questions I have are:

  1. Does the instance() method need to be synchronized?
  2. Is there a better way to handle violated workflow from the instance() method than throwing an exception?
  3. What are the dangers of call-back from under the lock?
\$\endgroup\$
8
  • \$\begingroup\$ Doesn't relate to your questions, so I'll post it as a comment: static std::unique_ptr<Config> cfg should be a static member of Config::init to avoid issues with the initialization order of class static variables. \$\endgroup\$ Jan 28, 2014 at 16:07
  • \$\begingroup\$ @Massimiliano, I thought initialization order is a problem when one has dependencies between static members and they are in different compilation units. Am I missing something? I am not sure in what sense it can be a member of Config::init() and be accessible by the Config::instance() method. \$\endgroup\$ Jan 28, 2014 at 17:17
  • 1
    \$\begingroup\$ You didn't miss anything :-) I just wanted to point out (in case it was of interest) that you can't call safely Config::init() or Config::instance() in code that initializes non-local variables. With a few modifications you may remove this restriction. Of course the solution is not without drawbacks, as the helper function now merges two responsibilities. \$\endgroup\$ Jan 28, 2014 at 19:24
  • \$\begingroup\$ Makes sense. Not my case but good to know. \$\endgroup\$ Jan 28, 2014 at 20:59
  • \$\begingroup\$ @Massimiliano If cfg were static inside Config::init then when would its constructor be called: would its constructor be called as soon as Config::init is called? If you define cfg inside Config::init after the lock_guard statement, then would cfg's constructor be delayed until after the lock is acquired? \$\endgroup\$
    – ChrisW
    Jan 28, 2014 at 21:30

1 Answer 1

5
\$\begingroup\$

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<mutex> lock(foo_mutex);
// get the config
config.init("foo", [](string loc){ cout << "I don't care"; });

Thread 2:

config.init("foo", [](string loc){
    lock_guard<mutex> 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<std::mutex> 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.

\$\endgroup\$
5
  • \$\begingroup\$ OK, so callback is now gone. I did two versions - with all flow errors as exceptions and as return values. I like the return values version more. \$\endgroup\$ Jan 28, 2014 at 15:48
  • \$\begingroup\$ I edited the answer to show how to keep the callback if you want it. \$\endgroup\$
    – ChrisW
    Jan 28, 2014 at 15:57
  • \$\begingroup\$ Makes sense, good pattern - will note. Generally I use callbacks if a "detour" of logic is needed. In this case it only complicates things as control returns to the caller anyway very directly. The new code looks much simpler too. Why did I go with the callback I do not understand. \$\endgroup\$ Jan 28, 2014 at 16:01
  • \$\begingroup\$ @VladDidenko Your using shared_ptr looks like an improvement over unique_ptr. \$\endgroup\$
    – ChrisW
    Jan 28, 2014 at 16:03
  • \$\begingroup\$ The shared pointer version is at the github.com/didenko/parametrized_config_singleton/tree/fb46de commit. Now Config::init() returns bool for consumer to inspect. Instance is now stored privately in a shared pointer, which makes the Config::instance() a very simply accessor - and the consumer can inspect if it is still nullptr or not. \$\endgroup\$ Sep 10, 2015 at 5:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.