3

I am struggling to get this working but it seems there is something I don't fully understand.

I have a multi-threaded application in a single file (2 classes and the real main in which I create threads and everything). I want to split in 3 files. One for each class (with their headers) and one for main.

I have a couple of mutices and a queue to share messages between threads and lock and unlock them at any time.

I want to write a singleton class so all of them can share the mutex:

header:

#ifndef COMMONSINGLETON_H
#define COMMONSINGLETON_H

#include <mutex>          // std::mutex
#include <queue>

class CommonSingleton{
    private:
        static std::mutex               mutex_w_b;
        //static std::mutex               mutex_wifi_ok;
        //static std::queue <std::string> queue_w_b;

    public:
        static CommonSingleton& getInstance();
        std::mutex GetMutexWB();
        //std::mutex GetMutexWiFiOk();
        //std::queue <std::string> GetQueueWB();
    private:
        CommonSingleton() {};
        CommonSingleton(CommonSingleton const&);
        void operator=(CommonSingleton const&);
};
#endif

And my .cpp file:

#include "commonsingleton.h"

static CommonSingleton& CommonSingleton::getInstance(){
    static CommonSingleton instance;
    return instance;
}
std::mutex CommonSingleton::GetMutexWB(){
    return mutex_w_b;
}
/*std::mutex CommonSingleton::GetMutexWiFiOk(){
    return mutex_wifi_ok;
}
std::queue <std::string> CommonSingleton::GetQueueWB(){
   return queue_w_b;
}*/

In my main I have this to test mutex:

CommonSingleton::getInstance().GetMutexWB.lock();

But now I have commented, just to figure out how to compile. This is the error I get:

commonsingleton.cpp:4:54: error: cannot declare member function ‘static 
CommonSingleton& CommonSingleton::getInstance()’ to have static linkage [-
fpermissive]
static CommonSingleton& CommonSingleton::getInstance(){
                                                  ^
commonsingleton.cpp: In member function ‘std::mutex 
CommonSingleton::GetMutexWB()’:
commonsingleton.cpp:9:12: error: use of deleted function 
‘std::mutex::mutex(const std::mutex&)’
return mutex_w_b;
            ^
In file included from commonsingleton.h:5:0,
from commonsingleton.cpp:1:
/usr/include/c++/4.8/mutex:128:5: error: declared here
mutex(const mutex&) = delete;

What am I doing wrong right here?

3
  • First google result for the static error
    – dydil
    Commented May 3, 2017 at 11:21
  • You must not return std::mutex by value, but by reference.
    – Walter
    Commented May 3, 2017 at 11:25
  • With both results I almost got it working. Now I get this error: /tmp/ccVyBCBk.o: In function CommonSingleton::GetMutexWB()': commonsingleton.cpp:(.text+0x47): undefined reference to CommonSingleton::mutex_w_b' collect2: error: ld returned 1 exit status
    – Biribu
    Commented May 3, 2017 at 11:30

4 Answers 4

6

You don't need so much boilerplate. You just need a function:

std::mutex& getCommonMutex() {
    static std::mutex m;
    return m;
}

That's all, you can call this function from anywhere and it will always return a reference to the same mutex, which will be initialized the first time its called.

1
  • My vote. Singleton classes is typically a cross-training from Java habit. C++ still has file-level functions and long may they live. A file-level std::mutex is a Python-esque (language not Monty) Global Interpreter Lock (GIL) in the making and could become a similar bottle-neck but maybe that's OK here.
    – Persixty
    Commented May 3, 2017 at 12:39
2

You have a few compiler errors as below:

Firstly, you don't need to write static again in the function definition, thus in your cpp:

CommonSingleton& CommonSingleton::getInstance(){
    static CommonSingleton instance;
    return instance;
}

Secondly, std::mutex has deleted copy and move constructors so you cannot return an existing mutex without passing it by reference (or const-ref) which converts your cpp functions to:

std::mutex& CommonSingleton::GetMutexWB(){
    return mutex_w_b;
}
std::mutex& CommonSingleton::GetMutexWiFiOk(){
    return mutex_wifi_ok;
}

Make sure you change these in the declaration as well (.h)

2

A more useful singleton is one in which client programs do not know that it's a singleton.

This means you can change your mind at a later date if you wish with no refactoring.

example:

#include <mutex>          // std::mutex
#include <iostream>
#include <thread>
#include <chrono>
#include <future>

// Note: This class is a singleton by internal implementation, not
// by interface. This separates the concerns of its function and its
// form, meaning client programs do not need to care.
//
class CommonSingleton{
  using mutex_type = std::mutex;
  using lock_type = std::unique_lock<mutex_type>;

private:
  struct Impl {

    mutex_type mutex_w_b;

    lock_type get_lock() { return lock_type(mutex_w_b); }

  };

  static Impl& get_impl() {
    static Impl impl {};
    return impl;
  }

public:

  lock_type get_lock() {
    return get_impl().get_lock();
  }
};


// note - copyable
void foo(CommonSingleton singleton)
{
  using namespace std::literals;
  for(int i = 0 ; i < 10000 ; ++i) {
    auto lock = singleton.get_lock();
    std::cout << "foo\n";
    lock.unlock();
    std::this_thread::sleep_for(1ms);
  }
}

void bar(CommonSingleton singleton)
{
  using namespace std::literals;
  for(int i = 0 ; i < 10000 ; ++i) {
    auto lock = singleton.get_lock();
    std::cout << "bar\n";
    lock.unlock();
    std::this_thread::sleep_for(1ms);
  }
}

void baz(CommonSingleton singleton)
{
  using namespace std::literals;
  for(int i = 0 ; i < 10000 ; ++i) {
    auto lock = singleton.get_lock();
    std::cout << "baz\n";
    lock.unlock();
    std::this_thread::sleep_for(1ms);
  }
}

int main()
{
  // all using the same mutex in reality...
  auto ffoo = std::async(std::launch::async, foo, CommonSingleton());
  auto fbar = std::async(std::launch::async, bar, CommonSingleton());
  auto fbaz = std::async(std::launch::async, baz, CommonSingleton());

  ffoo.wait();
  fbar.wait();
  fbaz.wait();
}
1

Keyword static has many different meanings in c++. When you use it in class member declaration you declare class member as static, but when you use it in class member definition in your .cpp file you force static linkage on it (while it should have external linkage to be usable in different translation units).

so in .cpp file you should change

static CommonSingleton& CommonSingleton::getInstance(){

to

CommonSingleton& CommonSingleton::getInstance(){

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.