What is a valid way to stop a std::thread by an external signal?

Question

This is a code which works not as designed please explain me what is wrong here (code simplified to make this more readable).

shm_server server;

std::thread s{server};

// some work...

std::cout << "press any key to stop the server";
_getch();
server.stop();
s.join();

It looks like I call a stop method for another copy of shm_server class. because stop() only sets std::atomic_bool done; (shm_server member) to true but I see that thread function (this is operator() of shm_server) still sees done equal to false.

std::thread has only move contructor?

How to send a signal to server correctly in this typical case?

class shm_server {
    std::atomic_bool    done;
public:
    shm_server() : done{false} {}
    shm_server(shm_server& ) : done{false} {}
    void operator()() {
       while (!done) {
       //...
       }
    }
    void stop() {
       done = true;
    }
};

Try `std::thread s{std::ref(server)};` to avoid the copying – Ben Voigt Jan 07 '16 at 21:49 — Ben Voigt, Jan 07 '16 at 21:49
related: http://stackoverflow.com/q/21048906/103167 – Ben Voigt Jan 07 '16 at 21:50 — Ben Voigt, Jan 07 '16 at 21:50
thank you! std::ref solution works for me – amigo421 Jan 07 '16 at 21:59 — amigo421, Jan 07 '16 at 21:59

5gon12eder · Accepted Answer · 2016-01-07T23:02:58.570

You have somehow shot yourself into the foot by making shm_server copyable. I believe you did this in order to get rid of the compiler error, not because copy semantics on that class are particularly useful. I'd recommend you delete that constructor again. If you really want copy semantics, have the constructor take its argument by reference to const.

class shm_server  // cannot copy or move
{

  std::atomic_bool done_ {};

public:

  void
  operator()()
  {
    this->run();
  }

  void
  run()
  {
    using namespace std::chrono_literals;
    while (!this->done_.load())
      {
        std::clog << std::this_thread::get_id() << " working..." << std::endl;
        std::this_thread::sleep_for(1ms);
      }
  }

  void
  stop()
  {
    this->done_.store(true);
  }

};

I have factored out the logic of the call operator into a named function for convenience. You don't need to do that but I'll use it later in the example.

Now we somehow have to make the run member function be executed on its own thread. If you write

shm_server server {};
std::thread worker {server};  // won't compile

the compiler won't let you do it as it – as you have conjectured yourself – would attempt to copy the server object which isn't copyable.

@Ben Voigt has suggested to wrap the server object into a std::reference_wrapper which is an object that behaves like a reference.

shm_server server {};
std::thread worker {std::ref(server)};

This works as intended and effectively passes a pointer to the std::thread constructor, rather than the actual server object.

However, I find that this is not the most straight-forward solution here. What you really want to do is run a member function of your server on a different thread. And std::thread's constructor lets you do just that.

shm_server server {};
std::thread worker {&shm_server::run, &server};

Here, I'm passing the address of the run member function and a pointer to the server object (that will be passed as the implicit this pointer) as argument. I have introduced the run function since the syntax might be less irritating. You can also pass the address of the call operator directly.

shm_server server {};
std::thread worker {&shm_server::operator(), &server};

Here is a complete example.

int
main()
{
  using namespace std::chrono_literals;
  std::clog << std::this_thread::get_id() << " starting up" << std::endl;
  shm_server server {};
  std::thread worker {&shm_server::operator(), &server};
  //std::thread worker {std::ref(server)};                 // same effect
  //std::thread worker {&shm_server::run, &server};        // same effect
  std::this_thread::sleep_for(10ms);
  server.stop();
  worker.join();
  std::clog << std::this_thread::get_id() << " good bye" << std::endl;
}

Possible output:

140435324311360 starting up
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435306977024 working...
140435324311360 good bye

If you believe that a shm_server will only ever be useful if run on its own thread, you can also give it a std::thread as data member and have its constructor (or a dedicated start member function, if you prefer) start and its destructor join the thread.

#include <atomic>
#include <chrono>
#include <iostream>
#include <thread>

class shm_server
{

  std::atomic_bool done_ {};
  std::thread worker_ {};

public:

  shm_server()
  {
    this->worker_ = std::thread {&shm_server::run_, this};
  }

  ~shm_server()
  {
    this->done_.store(true);
    if (this->worker_.joinable())
      this->worker_.join();
  }

private:

  void
  run_()
  {
    using namespace std::chrono_literals;
    while (!this->done_.load())
      {
        std::clog << std::this_thread::get_id() << " working..." << std::endl;
        std::this_thread::sleep_for(1ms);
      }
  }

};


int
main()
{
  using namespace std::chrono_literals;
  std::clog << std::this_thread::get_id() << " starting up" << std::endl;
  {
    shm_server server {};
    std::this_thread::sleep_for(10ms);
  }
  std::clog << std::this_thread::get_id() << " good bye" << std::endl;
}

thank you for detailed explanation. yes, I've added copy ctor without real analyzing the problem inside , just saw the problem with a copy atomic_bool. and after advice with std::ref, I've removed this as unused.. regarding member passing. I used an object itself not for some reason just it looks clear than function passing, so really , member passing is ok for the functionality. and question about thread object inside the server: is this pattern more preferable? any advanteges? — amigo421, Jan 08 '16 at 07:00
Encapsulating the threading logic inside your server class frees your clients from having to worry about it. On the other hand, it makes the design less flexible. Maybe some clients want to run it on their own thread or use another threading library. Also think about unit-testability. I don't know what will fit your use-case well. — 5gon12eder, Jan 08 '16 at 07:17
thank you,encapsulating is preferable for me, client wants to "create and forget" about server object. ideally, server should manage all completely by itself — amigo421, Jan 08 '16 at 07:25

What is a valid way to stop a std::thread by an external signal?

1 Answers1