boost::asio::ssl access violation on multi-threaded application

Question

I've created a program that makes use of boost's ssl implementation to send and receive small packets to a remote server using async_read_some() and async_write_some(). The read and write are wrapped in a strand to prevent concurrent reading and writing. Additionally the wrapper functions I've created for each contain a mutex to further prevent concurrent access (possibly overkill, but can't hurt). The writes are stored on a write queue where they are sent when the thread is notified that data is available.

The issue I'm experiencing occurs when a large number of writes are executed sequentially, resulting in varying errors such as Second Chance Assertion Failed and Access Violation. I also get a read error of "decryption failed or bad record mac".

From the research I've done so far, I've found that SSL sockets can become corrupted if reads and writes are performed concurrently - at least according to the discussions here and here where the OP's symptoms are very similar to mine. He also states that the strand he was using was not functioning, but I don't understand his solution. This would make sense with the issues I'm having due to the methods I'm attempting to use to prevent concurrent reads and writes.

A workaround I've been using is to throttle the sequential writes so that there's a gap of at least 20ms between each. Any less than this and I start getting errors. This workaround isn't great though, because at some point there may still be a concurrent read/write resulting in errors.

Here's a summary of my read/write code:

void client::write(std::string message, char packetType) {
    boost::lock_guard<boost::shared_mutex> lock(writeInProgressMutex);

    std::string preparedMessage = prepareWrite(message, packetType);
    char data_sent[2048];

    for(std::string::size_type i = 0; i < preparedMessage.size(); ++i) {
        data_sent[i] = preparedMessage[i];
        if (i + 1 == preparedMessage.size()) {
            data_sent[i+1] = NULL;
        }
    }

    socket_->async_write_some(boost::asio::buffer(data_sent), strand_.wrap(boost::bind(&client::handle_write, this, boost::asio::placeholders::error)));
}

void client::read() {
    boost::lock_guard<boost::shared_mutex> lock(readInProgressMutex);
    socket_->async_read_some(boost::asio::buffer(data_), strand_.wrap(boost::bind(&client::handle_read, this, boost::asio::placeholders::error)));
}

I've experimented with different types of mutexes, so I don't think that is the issue. If anyone knows a way to ensure my strand is doing its job or you can see some obvious errors in my code/design please let me know!

Answer 1

In short, verify that all calls to client::write() and client::read() are running within strand_. Without additional code, synchronization constructs, such as a mutex, will be ineffective for preventing concurrent operations from being invoked.

---

The Boost.Asio documentation for ssl::stream thread safety accentuates the strand requirement:

Distinct objects: Safe.

Shared objects: Unsafe. The application must also ensure that all asynchronous operations are performed within the same implicit or explicit strand.

If the ssl::stream operations are only being invoked within a single thread, then it is safe as they are running within an implicit strand. On the other hand, if multiple threads are invoking operations on an ssl::stream, then they must be explicitly synchronized with a strand. Other synchronization constructs, such as a mutex, will be ineffective due to intermediate handlers without introducing custom asio_handler_invoke functions.

Here is a flow diagram showing the necessary strand usage for an asynchronous write chain:

void client::start_write_chain()
{
  strand_.post(boost::bind(&client::write, ...)) ---.
}    .----------------------------------------------'
     |  .-------------------------------------------.
     V  V                                           |
void client::write(...)                             |
{                                                   |
  ...                                               |
  socket_->async_write_some(buffer, strand_.wrap(   |
    boost::bind(&client::handle_write, ...)));  --. | 
}    .--------------------------------------------' |
     V                                              |
void client::handle_write(                          |
  boost::system::error_code& error,                 |
  std::size_t bytes_transferred)                    |
{                                                   |
  // If there is still data remaining.              |
  if (bytes_transferred < all_data)                 |
    write(...);  -----------------------------------'
}

This requirement is the result of an implementation detail that ssl::stream asynchronous operations are implemented in terms of composed operations. The initial operation may occur within the context of the caller, and thus client::write() needs to be invoked from within strand_. These composed operations may result in many intermediate calls to socket_. With these intermediate calls having no knowledge of writeInProgressMutex, the mutex becomes ineffective for protecting socket_. Consider reading this answer for more details on composed operations and strands.

Additionally, within client::write(), the lifetime of the buffer data_sent fails to meet requirement for ssl::stream::async_write_some(), which states:

Although the buffers object may be copied as necessary, ownership of the underlying buffers is retained by the caller, which must guarantee that they remain valid until the handler is called.

In this case, data_sent's lifetime ends when client::write() returns, which may occur before the completion handler has been invoked.