9

I'm getting a weird error working on a project. I've created a super simple example to recreate the error.

I've created a class. What I'd like to do in this class, is to have a sort of 'getter' function for my class that fills values of a struct. In the main application, the user would instantiate this struct, pass it to a member function, and be able to read the values in the struct upon return. Because of the design of the actual class, this has to happen in a separate thread. Here's what I have:

myClass.h:

#ifndef __MY_CLASS_H__
#define __MY_CLASS_H__

#include <mutex>

class myClass {
public:
    struct my_struct_s {
        int field1;
        short field2;
    };

    int get_data(my_struct_s & my_struct);

private:

};

#endif /* __MY_CLASS_H__ */

myClass.cpp:

#include "myClass.h"

int myClass::get_data(struct my_struct_s & my_struct)
{
    int var1 = 5;
    char var2 = 2;

    my_struct.field1 = var1;
    my_struct.field2 = var2;

    return 0;
}

Main.cpp:

#include "myClass.h"
#include <iostream>
#include <thread>
#include <Windows.h>

bool thread_running;
std::thread thread;

void run_thread(myClass & lmyClass)
{
    myClass::my_struct_s my_struct;

    while (thread_running) {
        lmyClass.get_data(my_struct);

        std::cout << my_struct.field1 << std::endl;
        std::cout << my_struct.field2 << std::endl;

        Sleep(100);
    }
}

int main(int argc, char *argv[])
{
    myClass lmyClass; 

    thread_running = true;
    thread = std::thread(run_thread, lmyClass);

    Sleep(1000);
    thread_running = false;

    if (thread.joinable()) {
        thread.join();
    }

    getchar();

    return 0;
}

It works as expected. However, because of the asynchronous nature of the class, I need mutexes to protect data being handled in different threads within the class.

If I add a std::mutext as a private member of my class, I receive the following when I try to run the code:

Error 1 error C2280: 'std::mutex::mutex(const std::mutex &)' : attempting to reference a deleted function ...

1) I'm trying to understand why I'm receiving this error.

2) (this part is a little more opinion-based), given the information, is this 'getter' way of filling out a public struct so that someone implementing my class can mess with the variables within it a good design? Is there a better way of doing it?

justynnuff
  • 461
  • 1
  • 6
  • 20
  • 1
    Please show the actual code that is failing when trying to use a `std::mutex`. – Remy Lebeau Feb 03 '15 at 23:56
  • 1
    As already was said, std::mutex is not copyable. And it is no movable either. But you could have unique_ptr to mutex, if you want. And while at it, I'll make thread_running into atomic – Severin Pappadeux Feb 04 '15 at 00:05
  • @SeverinPappadeux If I instead make the instantiation of `lmyClass` global in Main.cpp, and access this object in my thread instead of passing it as an argument, it works. Which solution is better? – justynnuff Feb 04 '15 at 00:17
  • 1
    not sure I understand: if it is global, then it is more like singleton, but if you access it in one thread, then why bother with mutex/locking? Could you publish/send the code? And while we're at it, you might want to replace Sleep(100) with std::this_thread::yield(), if there is nothing to do, give up time slice voluntarely – Severin Pappadeux Feb 04 '15 at 03:36
  • The class itself has a private thread that is always reading from the serial port. It has public members like `start()` `stop()` `set_mode()` which send a message over the serial port, but the reading thread never stops. It just processes response frames once it knows what they are. Once frames are known, they're thrown into queues, which is where I'd like my 'getter' functions to pull data from. Which necessitates the need for mutexes, obviously (asynchronous queue access). – justynnuff Feb 04 '15 at 04:10
  • However, from an outside point of view, like from the perspective of the Main.cpp application, I need to access these getter functions in separate threads. Basically, the class has a reader thread, and mutex locks, but from outside the class, I need to access different getters in different threads, if this all makes sense. I don't want to post the code, it's far far too long, so hopefully I'm explaining it somewhat coherently. – justynnuff Feb 04 '15 at 04:10
  • 1
    In such case, (reader thread and processing thread(s)) you might want to use conditional variable. As an simple example (though I would change it to use atomic), http://en.cppreference.com/w/cpp/thread/condition_variable or discussion here: http://stackoverflow.com/questions/13371020/using-stdmutex-stdcondition-variable-and-stdunique-lock – Severin Pappadeux Feb 04 '15 at 18:11

2 Answers2

16

You don't have a copy constructor in myclass, so the compiler gives it a default copy constructor. This will attempt to copy all members, including your std::mutex, which is not copyable. As the compiler error says, it's marked as deleted.

You need to define your own copy constructor; most likely you'll want it to acquire the mutex stored in the instance being copied, then copy all of it's other members to the new instance.

Collin Dauphinee
  • 13,664
  • 1
  • 40
  • 71
2

Declare your mutex dynamically and not statically.

At your myClass class header declare the mutex as a pointer, like that:

mutex * mtx;

and at your constructor initialize the mutex calling his constructor:

mtx = new std::mutex();
Derzu
  • 7,011
  • 3
  • 57
  • 60