1

In the following class a worker thread is started within constructor. The worker has a blocking call to a queue.

It works as expected, but when the AsyncQueue object gets out of scope (for whatever reason), its destructor is called. Also, the destructor of the simple_queue object is called (what I checked by debugging).

But what happens to the worker? Because it is still waiting on the blocking call to the queue!

I observed, that without calling impl_thread_.detach(); within the destructor execution crashes. However, I don't know whether this is a solution at all. What I don't understand additionally: although the queue object is destroyed the blocking call is not raising an exception - in fact I set a break-point in the catch handler. So what is going on here and what is the right way to implement that scenario? I deeply feel, that what I do here is not quite as it should be ;-)

    template<typename T>
    class AsyncQueue
    {
    public:
        AsyncQueue() : impl_thread_(&AsyncQueue::worker, this)
        {
            tq_ = std::shared_ptr<simple_queue<T>>(new simple_queue<T>);
            impl_thread_.detach();
        }
        //~AsyncQueue() = default;

        ~AsyncQueue() {
            std::cout << "[" << boost::this_thread::get_id() << "] destructor AsyncQueue" << std::endl;
            return;
        }

    private:
        std::thread impl_thread_;
        std::shared_ptr<simple_queue<T>> tq_;

        void worker()
        {
            try {
                while (true)
                {
                    boost::optional<T> item = tq_->deq(); // blocks

                    ...
                    ...
                    ...
                }

            }
            catch (exception const& e) {
                return;
            }
        }

    public:
    ...
    ...


    };
MichaelW
  • 1,328
  • 1
  • 15
  • 32
  • 3
    Question OT: I don't think this is a safe way to start a thread: `: impl_thread_(&AsyncQueue::worker, this)`. The problem is that, if an exception is later thrown in the constructor (such as from the `new`), the destructor of _joinable_ thread `impl_thread_` may be called, which results in `std::terminate`. – Daniel Langr Feb 07 '20 at 09:05
  • 2
    Don't add code that is without any value (those two returns in destructor and catch block). You'll return from function anyway as soon as last expression is run. – Aconcagua Feb 07 '20 at 09:37
  • 1
    Your thread starts and accesses `tq_` before that member is guaranteed to be set as well. Also, is there a particual reason why `tq_` is a shared pointer? Just use a plain object. When declared before `impl_thread_`, you also solve the race condition. – Ulrich Eckhardt Feb 07 '20 at 10:53
  • @Ulrich: Yes, you are right. Modifying the constructor, however, doesn't solve my problem, which is more related to destructing. Using plain object instead of shared pointer neither. – MichaelW Feb 07 '20 at 18:25

1 Answers1

2

The simplest way if you can is in your destructor to send a stop token in your eq queue and check on the stop token in the worker to exit it. Remove the detach first.

~AsyncQueue() {
  _eq.add(stopToken); // what ever your can use here. else use an atomic bool 
  std::cout << "[" << boost::this_thread::get_id() << "] destructor AsyncQueue" << std::endl;
  impl_thread_.join();
}

(untested, incomplete)

Surt
  • 15,501
  • 3
  • 23
  • 39