Stopping boost::asio::io_service::run() from concurrent destructor

Question

Can anybody explain me why this program does not terminate (see the comments)?

#include <boost/asio/io_service.hpp>
#include <boost/asio.hpp>
#include <memory>
#include <cstdio>
#include <iostream>
#include <future>

class Service {
public:
    ~Service() {
        std::cout << "Destroying...\n";
        io_service.post([this]() {
            std::cout << "clean and stop\n"; // does not get called
            // do some cleanup
            // ...
            io_service.stop();
            std::cout << "Bye!\n";
        });
        std::cout << "...destroyed\n"; // last printed line, blocks
    }

    void operator()() {
        io_service.run();
        std::cout << "run completed\n";
    }

private:
    boost::asio::io_service io_service;
    boost::asio::io_service::work work{io_service};
};

struct Test {
    void start() {
        f = std::async(std::launch::async, [this]() { service(); std::cout << "exiting thread\n";});
    }
    std::future<void> f;
    Service service;
};

int main(int argc, char* argv[]) {
    {
        Test test;
        test.start();

        std::string exit;
        std::cin >> exit;
    }

    std::cout << "exiting program\n"; // never printed
}

Answer 1

The real issue is that destruction of io_service is (obviously) not thread-safe.

Just reset the work and join the thread. Optionally, set a flag so your IO operations know shutdown is in progress.

You Test and Service classes are trying to share responsibility for the IO service, that doesn't work. Here's much simplified, merging the classes and dropping the unused future.

Live On Coliru

The trick was to make the work object optional<>:

#include <boost/asio.hpp>
#include <boost/optional.hpp>
#include <iostream>
#include <thread>

struct Service {
    ~Service() {
        std::cout << "clean and stop\n";
        io_service.post([this]() {
            work.reset(); // let io_service run out of work
        });

        if (worker.joinable())
            worker.join();
    }

    void start() {
        assert(!worker.joinable());
        worker = std::thread([this] { io_service.run(); std::cout << "exiting thread\n";});
    }

private:
    boost::asio::io_service io_service;
    std::thread worker;
    boost::optional<boost::asio::io_service::work> work{io_service};
};

int main() {
    {
        Service test;
        test.start();

        std::cin.ignore(1024, '\n');
        std::cout << "Start shutdown\n";
    }

    std::cout << "exiting program\n"; // never printed
}

Prints

Start shutdown
clean and stop
exiting thread
exiting program

Answer 2

See here: boost::asio hangs in resolver service destructor after throwing out of io_service::run()

I think the trick here is to destroy the worker (the work member) before calling io_service.stop(). I.e. in this case the work could be an unique_ptr, and call reset() explicitly before stopping the service.

EDIT: The above helped me some time ago in my case, where the ioservice::stop didn't stop and was waiting for some dispatching events which never happened.

However I reproduced the problem you have on my machine and this seems to be a race condition inside ioservice, a race between ioservice::post() and the ioservice destruction code (shutdown_service). In particular, if the shutdown_service() is triggered before the post() notification wakes up the other thread, the shutdown_service() code removes the operation from the queue (and "destroys" it instead of calling it), therefore the lambda is never called then.

For now it seems to me that you'd need to call the io_service.stop() directly in the destructor, not postponed via the post() as that apparently doest not work here because of the race.

Answer 3

I was able to fix the problem by rewriting your code like so:

class Service {
public:
    ~Service() {
        std::cout << "Destroying...\n";
        work.reset();
        std::cout << "...destroyed\n"; // last printed line, blocks
    }

    void operator()() {
        io_service.run();
        std::cout << "run completed\n";
    }

private:
    boost::asio::io_service io_service;
    std::unique_ptr<boost::asio::io_service::work> work = std::make_unique<boost::asio::io_service::work>(io_service);
};

However, this is largely a bandaid solution.

The problem lies in your design ethos; specifically, in choosing not to tie the lifetime of the executing thread directly to the io_service object:

struct Test {
    void start() {
        f = std::async(std::launch::async, [this]() { service(); std::cout << "exiting thread\n";});
    }
    std::future<void> f; //Constructed First, deleted last
    Service service; //Constructed second, deleted first
};

In this particular scenario, the thread is going to continue to attempt to execute io_service.run() past the lifetime of the io_service object itself. If more than the basic work object were executing on the service, you very quickly begin to deal with undefined behavior with calling member functions of deleted objects.

You could reverse the order of the member objects in Test:

struct Test {
    void start() {
        f = std::async(std::launch::async, [this]() { service(); std::cout << "exiting thread\n";});
    }
    Service service;
    std::future<void> f;
};

But it still represents a significant design flaw.

The way that I usually implement anything which uses io_service is to tie its lifetime to the threads that are actually going to be executing on it.

class Service {
public:
    Service(size_t num_of_threads = 1) :
        work(std::make_unique<boost::asio::io_service::work>(io_service))
    {
        for (size_t thread_index = 0; thread_index < num_of_threads; thread_index++) {
            threads.emplace_back([this] {io_service.run(); });
        }
    }

    ~Service() {
        work.reset();
        for (std::thread & thread : threads) 
            thread.join();
    }
private:
    boost::asio::io_service io_service;
    std::unique_ptr<boost::asio::io_service::work> work;
    std::vector<std::thread> threads;
};

Now, if you have any infinite loops active on any of these threads, you'll still need to make sure you properly clean those up, but at least the code specific to the operation of this io_service is cleaned up correctly.