TCP retarder for network delay simulation

I would like to simulate delay over network, for that I have implemented a simple retarder (=introduce random delay while maintaining order of messages) using Boost to handle sending itself. It appears to work as intended. However, since parallel programming is not trivial especially when coupled with networking, I would like to hear feedback more experienced C++ programmer. Mostly, I am concerned about the parallelization(deadlocks, resource leaks) and possible side effects on the communication I might have missed.

class SendRetarder
{
    typedef std::pair args_t;//to whom and what
    std::queue message_queue;//messages to be sent
    std::mutex m;
    std::condition_variable cv;
    const size_t max_wait = 10000;

    void send_loop()
    {
        while (true)
        {
            boost::system::error_code ec;
            args_t args;
            size_t wait;
            {
                std::unique_lock lk(m);
                cv.wait(lk, [&] { return !message_queue.empty(); });
                wait = (rand() % max_wait) / message_queue.size();//to prevent unchecked growing
                args = message_queue.front();
                message_queue.pop();
            }
            std::this_thread::sleep_for(std::chrono::milliseconds(wait));
            boost::asio::write(args.first->socket_, boost::asio::buffer(&args.second, sizeof(message_t)), ec);
            if (ec) std::cerr  lk(m);
            message_queue.push(args_t(client, msg));
        }
        cv.notify_one();
    }
};

where message_t is the contents of the message consisting of integer types and session_ptr is a smart pointer wrapper around socket shared with client itself:

struct message_t {
    msg_id_t message_id;
    tag_t message_tag;
    sender_t sender_id;
    union { uint32_t value; bool success; } data;
}

typedef std::shared_ptr session_ptr;
class Session : public std::enable_shared_from_this {
    tcp::socket socket_;
    sender_t name_;
    //...
}

I see a number of things that may help you improve your code.

Consider using existing tools

First, let me note that I've had a need for a similar thing before, but what I eventually used was tc which was tremendously useful for my testing and required essentially no additional code on my part. In my case, although neither of the devices I was testing ran Linux, I put a Linux box between the devices and did all of the data logging from there. Not only did this allow me to arbitrarily delay some number of packets, but also allowed the deliberate injection of bit errors, etc. which very much expanded the possible test repertoire.

Make sure you have all required #includes

The code uses a number of things from both the standard library and from boost but doesn't show the required #includes. Carefully consider which #includes are part of the interface (and belong in the .h file) and which are part of the implementation and provide them to users (and reviewers!) of the code.

Lock shared resources before use

The send_loop() routine prints any error message to std::cerr but does so without a lock. This is a problem because std::cerr is a shared resource. If this thread is in the middle of printing a message when another thread writes, you may get interleaved messages. While this is not fatal, it's easily avoided by controlling access to std::cerr via a mutex.

Don't launch a thread without considering object lifetime

The SendRetarder constructor simply launches a detached thread with this line:

std::thread(&SendRetarder::send_loop, this).detach();

However, this has a major problem. What if the object is destroyed after this thread is launched? Everything within the send_loop() routine that relies on object variables is going to be potentially using de-allocated memory and non-existent objects! Prevent this by using shared_from_this and a std::shared_ptr instead of just passing this.

Provide a destructor

What happens to the detached thread if the object is destroyed? Nothing, at the moment, and that's a problem. If the object is destroyed, it seems to me that either there should be a way to signal the detached thread that it's time to quit. One simple way to do that is to close the underlying socket which is one thing that should probably be done in a destructor.

Consider using a better random number generator

If you are using a compiler that supports at least C++11, consider using a better random number generator. In particular, instead of rand, you might want to look at std::uniform_int_distribution and friends in the ` header.

Consider future users

The line that writes the message to the socket looks like this:

boost::asio::write(args.first->socket_,
                   boost::asio::buffer(&args.second, sizeof(message_t)), ec);

That works right now, but what happens if you decide to have a variable size message? For example, consider this:

using message_t = std::string;

Now the sizeof() operator will probably return the size of a pointer rather than the size of the actual object which will lead to maddenly difficult-to-find bugs. I'd recommend adding a size() method to your existing message_t object and then changing the write to look like this:

boost::asio::write(args.first->socket_, 
                   boost::asio::buffer(&args.second, args.second.size())), ec);

Then if the message_t is ever changed to something else, it will be required to implement a size() also. One way to enforce that, if needed, would be to have a message_t base class and have a pure virtual size()` but that may be overkill for your purposes.

StackExchange Code Review Q#154649, answer score: 3