How to avoid data race with `asio::ip::tcp::iostream`? How to avoid data race with `asio::ip::tcp::iostream`? multithreading multithreading

How to avoid data race with `asio::ip::tcp::iostream`?


Yeah, you're sharing the socket that underlies the stream, without synchronization

Sidenote, same with the boolean flags, which can easily be "fixed" by changing:

std::atomic_bool want_quit;std::atomic_bool want_reset;

How To Solve

To be honest, I don't think there is a good solution. You said it yourself: the operations are asynchronous, so you'll be in trouble if you try to do them synchronously.

You could try to think of hacks. What if we created a separate stream object based on the same underlying socket (filedescriptor). It's not going to be very easy as such a stream is not part of Asio.

But we could hack one up using Boost Iostreams:

#define BOOST_IOSTREAMS_USE_DEPRECATED#include <boost/iostreams/device/file_descriptor.hpp>#include <boost/iostreams/stream.hpp>// .... later:    // HACK: procure a _separate `ostream` to prevent the race, using the same fd    namespace bio = boost::iostreams;    bio::file_descriptor_sink fds(stream.rdbuf()->native_handle(), false); // close_on_exit flag is deprecated    bio::stream<bio::file_descriptor_sink> hack_ostream(fds);    con.run(stream, hack_ostream);

Indeed this runs without the race (simultaneous reads and writes on the same socket are fine, as long as you don't share the non-threadsafe Asio object(s) wrapping them).

What I Recommend Instead:

Don't do that. It's a kludge. You're complicating things, apparently in an attempt to avoid using asynchronous code. I'd bite the bullet.

It's not too much work to factor the IO mechanics out from the service logic. You'll end up being free from random limitations (you could consider dealing with multiple clients, you could do without any threading at all etc.).

If you would like to learn about some middle ground, look at stackful coroutines (http://www.boost.org/doc/libs/1_66_0/doc/html/boost_asio/reference/spawn.html)

Listing

Just for reference

Note I refactored to remove the need for pointers. You're not transferring ownership, so a reference will do. In case you didn't know how to pass the reference to a bind/std::thread constructor, the trick is in the std::ref you'll see.

[For stress testing I have greatly reduced the delays.]

Live On Coliru

#include <boost/asio.hpp>#include <iostream>#include <fstream>#include <thread>#include <array>#include <chrono>class Console {public:    Console() :        want_quit{false},        want_reset{false}    {}    bool getQuitValue() const { return want_quit; }    int run(std::istream &in, std::ostream &out);    bool wantReset() const { return want_reset; }private:    int runTx(std::istream &in);    int runRx(std::ostream &out);    std::atomic_bool want_quit;    std::atomic_bool want_reset;};int Console::runTx(std::istream &in) {    static const std::array<std::string, 3> cmds{        {"quit", "one", "two"},     };    std::string command;    while (!want_quit && !want_reset && in >> command) {        if (command == cmds.front()) {            want_quit = true;        }        if (std::find(cmds.cbegin(), cmds.cend(), command) == cmds.cend()) {            want_reset = true;            std::cout << "unknown command [" << command << "]\n";        } else {            std::cout << command << '\n';        }    }    return 0;}int Console::runRx(std::ostream &out) {    for (int i=0; !(want_reset || want_quit); ++i) {        out << "This is message number " << i << '\n';        std::this_thread::sleep_for(std::chrono::milliseconds(1));        out.flush();    }    return 0;}int Console::run(std::istream &in, std::ostream &out) {    want_reset = false;    std::thread t1{&Console::runRx, this, std::ref(out)};    int status = runTx(in);    t1.join();    return status;}#define BOOST_IOSTREAMS_USE_DEPRECATED#include <boost/iostreams/device/file_descriptor.hpp>#include <boost/iostreams/stream.hpp>int main(){    Console con;    boost::asio::io_service ios;    // IPv4 address, port 5555    boost::asio::ip::tcp::acceptor acceptor(ios, boost::asio::ip::tcp::endpoint{boost::asio::ip::tcp::v4(), 5555});    while (!con.getQuitValue()) {        boost::asio::ip::tcp::iostream stream;        acceptor.accept(*stream.rdbuf());        {            // HACK: procure a _separate `ostream` to prevent the race, using the same fd            namespace bio = boost::iostreams;            bio::file_descriptor_sink fds(stream.rdbuf()->native_handle(), false); // close_on_exit flag is deprecated            bio::stream<bio::file_descriptor_sink> hack_ostream(fds);            con.run(stream, hack_ostream);        }        if (con.wantReset()) {            std::cout << "resetting\n";        }    }}

Testing:

netcat localhost 5555 <<<quitThis is message number 0This is message number 1This is message number 2

And

commands=( one two one two one two one two one two one two one two three )while sleep 0.1; do echo ${commands[$(($RANDOM%${#commands}))]}; done | (while netcat localhost 5555; do sleep 1; done)

runs indefinitely, occasionally resetting the connection (when command "three" has been sent).