-2

I am trying to make a program that uses shared resources, but all I get in the is std::logic_error. I think I am not using the mutex in the right way. Here is a snippet of the code.

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>

struct camera {
  std::string name;
  std::string mac;
  bool accessStatus;
};

class service {
public:
    service(){};
    void run();
private:
    mutable std::mutex _mutex;
};

void service::run()
{
   unsigned char option;

   // some dummy camera object
   camera camera_object;
   camera_object.name = "camera_name";
   camera_object.mac = "B6:24:3D:4C:00:9B";
   camera_object.accessStatus = true;


   // a vector of objects
   std::vector<camera> cameras;
   cameras.push_back(camera_object);

   std::thread TT([&](){
       while (true) {

           // dummy condition
           if (1 == 1) {
               std::cout << cameras.size();
           }

           {
             std::unique_lock<std::mutex> mlock(_mutex);
             std::cout << "Choose an option:\n"
                       << "\t 1. add one more camera \n"
                       << "\t 2. get the theme \n"
                       << std::flush;

             option = getchar();
             switch (option) {
                 case '1':
                     cameras.push_back(camera_object);
                     break;
                 case '2':
                     std::cout << "Not yet implemented\n" << std::flush;
                     break;
                 default:
                     std::cout << "Invalid input\n" << std::flush;
                     break;
             }


           }

           // don't waste CPU resources
           using namespace std::chrono_literals;
           std::this_thread::sleep_for(1s);
           system("clear");
       }
   });
  TT.detach();
}

int main() {
    service sv;
    sv.run();
    return 0;
}

Sometimes when I run it it just returns segmentation fault, but other times it let me choose an option, but after I choose it I get std::logic_error. I am trying to understand how mutex and multithreading works, but I have a hard time on this one.

Edit: the shared resource is the cameras vector. I am doing this program just to learn, it does not have a real objective. The condition 1==1 is there just to be sure that is always prints the vector size.

Gabi5537
  • 101
  • 9
  • 4
    why do you `detach` ? Why do you use a thread for this (waiting for user input) in the first place? Anyhow, a [mcve] is needed – 463035818_is_not_an_ai Dec 12 '22 at 10:46
  • what shared resources? Do you mean `std::cin` and `std::cout` ? – 463035818_is_not_an_ai Dec 12 '22 at 10:47
  • 2
    "Edit: the shared resource is the cameras vector." no. This vector cannot be shared between different threads, because each time you call the function a new vector is created and a new thread writing to that vector. But again: If you need help with the segfault or runtime error you need to post a [mcve] – 463035818_is_not_an_ai Dec 12 '22 at 10:53
  • " The condition 1==1 is there just to be sure that is always prints the vector size." you can remove that. `std::cout << cameras.size();` does already print the vectors size, always, sure – 463035818_is_not_an_ai Dec 12 '22 at 10:54
  • Is not this aleaready a mininal reprodubcbile example? The stop requested condition only waits for "ESC" to be pressed and camera is just a struct, this is all the code. I deatch because if I dont the result is always "terminate called without an active exception" – Gabi5537 Dec 12 '22 at 10:57
  • this cannot reproduce a segfault. When I copy this code and try to compile I will just get a couple of compiler errors that are not related to your question. From the [link](https://stackoverflow.com/help/minimal-reproducible-example): "…Reproducible – Test the code you're about to provide to make sure it reproduces the problem" – 463035818_is_not_an_ai Dec 12 '22 at 10:58
  • also, what is `stop_requested()` ? This looks like something that is actually shared. – 463035818_is_not_an_ai Dec 12 '22 at 11:01
  • I edited it, I hope it helps, I want the program to run until I stop it. I hope it makes more sense now – Gabi5537 Dec 12 '22 at 11:07

1 Answers1

2

Your problem isn't really the threading, it's the fact that your lambda captures by reference a cameras vector that goes out of scope and is destroyed. You can reproduce this deterministically even with a single thread:

std::function<void(void)> foo()
{
  std::vector<int> out_of_scope;

  return [&]() { out_of_scope.push_back(42); };
}

anywhere you call the returned std::function will have Undefined Behaviour, because the vector no longer exists. Invoking this UB in a different thread doesn't change anything.

If you're going to have shared state, you have to make sure it lives at least as long as the threads using it. Just make the cameras vector a member of service alongside the mutex that protects it. Or join the thread so the vector doesn't go out of scope until after the thread exits. Either will work.

Useless
  • 64,155
  • 6
  • 88
  • 132