lazy initialization of a static member inside a templated class

Question

Hi I'm pretty new in C++ and I'm tryng to implement a thread pool, that's because I'm tryng to fix some C++ concepts. Here it is what I've written so far:

#include <condition_variable>
#include <functional>
#include <future>
#include <mutex>
#include <optional>
#include <queue>

/**
 * This class should be a singleton, that's because if
 * we want to limit the amount of running threads
 * we should check that there are not multiple instances
 * of this class. This implementation provides guarantees
 * that there will be a single instance of a given template
 * specialization. This class provide also a thread safe queue.
 */

template <typename R, typename... Args> class ThreadPool {

private:
  std::mutex m;
  std::condition_variable cv;
  bool is_running;
  size_t pool_size;
  std::queue<std::packaged_task<R(Args...)>> jobs;
  static ThreadPool<R, Args...> *instance; // Singleton instance
  ThreadPool()
      : is_running{true}, pool_size{std::thread::hardware_concurrency()} {
  } // the constructor should not be accessible
  ThreadPool(size_t pool_size)
      : is_running{true}, pool_size{pool_size} {
  } // the constructor should not be accessible

public:
  ThreadPool(const ThreadPool &) = delete; // copy constructor disabled;
  ThreadPool &
  operator=(const ThreadPool &) = delete; // copy assignment disabled
  ThreadPool &
  operator=(ThreadPool &&other) = delete; // movement temporary disabled

  /**
   * Create a thred pool with a size that it's equals to
   * the number of cores on the machine. This istance
   * should be use to maximize the parallelism.
   */
  static ThreadPool<R, Args...> *getInstance() {
    if (instance == nullptr)
      instance = new ThreadPool<R, Args...>{};
    return ThreadPool::instance;
  }

  /**
   * Create a thred pool with a size that it's equals to
   * the given pool size. This istance should be use
   * when we don't need to use the highest level of
   * parallelism.
   *
   * @pool_size: desired size of the thread pool
   */

  static ThreadPool<R, Args...> *getInstance(const size_t pool_size) {
    if (ThreadPool::instance == nullptr)
      ThreadPool::instance = new ThreadPool<R, Args...>{pool_size};
    return ThreadPool::instance;
  }

  void submit(std::packaged_task<R(Args...)> f) {
    std::unique_lock l{m};
    if (is_running) {
      if (jobs.size() == pool_size)
        cv.wait(l, [&]() { return jobs.size() < pool_size; });
      jobs.push(std::move(f));
      cv.notify_one();
    }
  }

  std::optional<std::packaged_task<R(Args...)>> get() {
    std::unique_lock l{m};
    if (jobs.size() == 0 && is_running)
      cv.wait(l, [&]() { return jobs.size() > 0 || !is_running; });
    if (jobs.size() > 0 && is_running) {
      std::packaged_task<R(Args...)> f = std::move(jobs.front());
      cv.notify_one();
      return std::optional<std::packaged_task<R(Args...)>>{std::move(f)};
    }
    return std::nullopt;
  }

  void quit() {
    // todo: valutare eccezione su quit multiple
    std::unique_lock l{m};
    if (is_running) {
      is_running = false;
      cv.notify_all();
    }
  }
};

int main() {

  static ThreadPool<int, int, int> *t = ThreadPool<int, int, int>::getInstance();
 
  return 0;
}

When i compile: g++ -g -Wall -Werror -pthread -std=c++17 main.cpp -o main it gives me the following error:

/usr/bin/ld: /tmp/cc6zwABg.o: in function `ThreadPool<int, int, int>::getInstance()':
/home/gjcode/Politecnico/CodeProjects/PDS/cpp/threadpool/threadpool.h:54: undefined reference to `ThreadPool<int, int, int>::instance'
/usr/bin/ld: /home/gjcode/Politecnico/CodeProjects/PDS/cpp/threadpool/threadpool.h:55: undefined reference to `ThreadPool<int, int, int>::instance'
/usr/bin/ld: /home/gjcode/Politecnico/CodeProjects/PDS/cpp/threadpool/threadpool.h:56: undefined reference to `ThreadPool<int, int, int>::instance'
collect2: error: ld returned 1 exit status

It seems to be related to the linking phase, I've tried to use cppinsights to see how the tamplate is initialized and it seems to work properly. Why I cannot access the static member instance ?? I tried also to move the field in the public area but it doesn't work.

Answer 1

You've forgotten to define the instance outside the class definition:

template <typename R, typename... Args>
ThreadPool<R, Args...>* ThreadPool<R, Args...>::instance = nullptr;

Opinion: There's something odd about having two ways that an instance can get created. If you make the calls in this order:

static ThreadPool<R, Args...> *getInstance();
static ThreadPool<R, Args...> *getInstance(const size_t pool_size);

...you'll get one with the default pool_size and if you do it the other way around, you'll get a user defined pool_size. I suggest settling on one of them and move the instance into the actual getInstance() method. Instead of having two ways of instantiating the singleton, you could make the pool_size a template parameter.

If you'd like two keep the two ways of constructing it, you could do something like this:

static ThreadPool<R, Args...>& getInstance(const size_t pool_size) {
    static ThreadPool<R, Args...> instance(pool_size);
    return instance;
}
static ThreadPool<R, Args...>& getInstance() {
    return getInstance(std::thread::hardware_concurrency());
}

Note that it returns a reference instead. This way has some benefits:

• There's no need to delete it later (which you also have forgotten in your current implementation).
• You can remove the default constructor.
• There's no need to check if it's a nullptr.
• It's thread-safe (which your current implementation is not).