boost asio async_write with shared buffer over multi-thread

Question

Now I have a Connection class as shown below (irrelevant things are omitted):

class Connection : public std::enable_shared_from_this<Connection> {
public:
    virtual void write() {
        socket_->async_write_some(boost::asio::buffer(buffer_.data(),
                                                      buffer_.size()),
                                  std::bind(&Connection::on_written,
                                            shared_from_this(),
                                            std::placeholders::_1,
                                            std::placeholders::_2));
    }

    void on_written(const boost::system::error_code& e, std::size_t length) {
        if(e) {
            // handle error here
            return;
        }

        buffer_.consume(length);
    }

    void add_to_buf(const std::string& data) {
        // add the string data to buffer_ here
    }

private:
    boost::asio::io_service& service_;
    std::unique_ptr<socket> socket_;

    boost::asio::streambuf buffer_;
};

As you see, the write() operation will write data in buffer_ and the buffer_ is only cleaned in the write operation completion handler. However, the problem comes, now I have the following invocation code (Note: it is multi-threaded):

Connection conn;
// initialization code here
conn.add_to_buf("first ");
conn.write();
conn.add_to_buf("second");
conn.write();

The output I want is first second, but, sometimes the output could be first first second. It happens when the second operation starts but the first completion handler has not been called. I have read about strand to serialize things, however, it can only serialize tasks, it cannot serialize a completion handler and a task.

Someone may suggest to call the second write operation in the first's completion handler, but, per the design, this cannot be achieved.

So, any suggestions? maybe a lock on buffer_?

Answer 1

Locking the buffer per se wont change anything. If you call write before the first write has completed it will send the same data again. In my opinion the best way is to drop the add_to_buf method and just stick to a write function that does both, add data to the buffer and if neccessary triggers a send.

class Connection : public std::enable_shared_from_this<Connection> {
public:
    virtual void write(const std::string& data) {
        std::lock_guard<std::mutex> l(lock_);
        bool triggerSend = buffer_.size() == 0;
        // add data to buffer
        if (triggerSend) {
            do_send_chunk();
        }
    }

    void on_written(const boost::system::error_code& e, std::size_t length) {
        if (e) {
            // handle error here
            return;
        }

        std::lock_guard<std::mutex> l(lock_);
        buffer_.consume(length);
        if (buffer_.size() > 0) {
            do_send_chunk();
        }
    }

private:
    void do_send_chunk() {
        socket_->async_write_some(boost::asio::buffer(buffer_.data(),
            buffer_.size()),
            std::bind(&Connection::on_written,
            shared_from_this(),
            std::placeholders::_1,
            std::placeholders::_2));
    }

    boost::asio::io_service& service_;
    std::unique_ptr<socket> socket_;

    boost::asio::streambuf buffer_;
    std::mutex lock_;
};

The idea is, that the write function checks if there is still data left in the buffer. In this case it does not have to trigger a do_send_chunk call as sooner or later on_written will be called which then will cause another do_send_chunk as the new data will stay in the buffer and the if(buffer_.size() > 0) will be true inside on_written. If however there is no data left it has to trigger a send operation.