c++ declaring a mutex in a class with seperate header

Question

I am trying to get a simple network program to work with threads. The idea was to have two threads, one for sending and one for receiving data over a socket. The thread responsible for reading data would update a buffer so that the sender could get response messages to confirm actions etc. The program has a class called netctl.cpp and a header netctl.h. When i tried to declare the mutex that i was going to use for the buffer in the header i got a ton of errors. So instead i tried to pass a mutex pointer to the constructor of netctl and that worked.

Is this approach safe/good practice? Otherwise, how would i set it up for this scenario to work?

This works

 // launch.cpp

int main(){
    std::mutex mtex;
    Netctl network_control(&mtex);

    std::thread thread_listener(&Netctl::network_listener, network_control);
    thread_listener.join();
}

// netctl.cpp

#include "netctl.h"
Netctl::Netctl(std::mutex* m){
    mutex = m;
}

Netctl::~Netctl(){}

// netctl.h

#ifndef NETCTL_H
#define NETCTL_H

class Netctl{
    public:
        Netctl(std::mutex* m);
        ~Netctl();

    private:
        std::mutex* mutex;
};
#endif

I expected the example below to work. why does this not work? I understand that mutexes cannot be copied. But why can't i create the mutex in the constructor or in the header?

This does not work

// launch.cpp

int main(){

    Netctl network_control();

    std::thread thread_listener(&Netctl::network_listener, network_control);
    thread_listener.join();
}

// netctl.cpp

#include "netctl.h"
Netctl::Netctl(){
    std::mutex mtex; // tried declaring mutex here
}
Netctl::~Netctl(){}

// netctl.h

#ifndef NETCTL_H
#define NETCTL_H

class Netctl{
    public:
        Netctl();
        ~Netctl();

    private:
        std::mutex* mutex; // also tried it here
};
#endif

I'm not very experienced with c++ so i apologize if my question is confusing.

the `This does not work` part doesn't seems to have any shared `mutex`. — apple apple, Oct 20 '19 at 15:19
You fell for a classic: It's called the "most vexing parse of C++". Use that as search term. That said, as a new user here, take the [tour] and read [ask]. — Ulrich Eckhardt, Oct 20 '19 at 15:51
@appleapple the definition in the header file is not enough? does it have to be declared somewhere? Thank you for your answer! — ggg, Oct 22 '19 at 15:32
@UlrichEckhardt Interesting, never heard of this before! I did some reading to get a grasp of what it is, but where does it occur in my code? Are you referring to "Netctl network_control();" in main? Thank you for your answer! — ggg, Oct 22 '19 at 15:35
@ggg the *same instance* of mutex should be used in all thread. In `netctl.cpp` you construct a mutex and destoy it, by no chance it would be shared. In `.h` you simply declare a pointer so it would not work either. — apple apple, Oct 22 '19 at 16:09
Exactly that's what I meant, @ggg. `Netctl network_control();` declares a function. I think with modern C++ a better syntax is `Netctl network_control{};` instead. — Ulrich Eckhardt, Oct 22 '19 at 18:40

score 0 · Answer 1 · answered Oct 22 '19 at 15:29

After some research i came across someone having a similar problem https://stackoverflow.com/a/46295882/11795104. Adding std::ref solved my problem.

std::thread thread_listener(&Netctl::network_listener, std::ref(network_control));
std::thread thread_keyboard(&Netctl::user_input, std::ref(network_control));

c++ declaring a mutex in a class with seperate header

1 Answers1