c++asynchronousrequestboost-asiofuture

Client theoretically capable of sending multiple requests actually incapable of doing that, probably because it fails to manage async tasks correctly


I am dealing with code for a tcp client making asynchronous http requests.

The code uses boost::asio:

#include <boost/asio.hpp>
#include <future>
//...#include <..etc etc ..>

using ResolveResult = boost::asio::ip::tcp::resolver::results_type;
using Endpoint = boost::asio::ip::tcp::endpoint;

Quite canonically, the program defines a struct Request which handles the connection to server:

struct Request {
  // constructor
  explicit Request(boost::asio::io_context& io_context, std::string host)
      : resolver{ io_context }
      , socket{ io_context }
      , host{ std::move(host) } {/*...*/}

  // declarations of methods
  void resolution_handler(boost::system::error_code ec, const ResolveResult& results);
  void connection_handler(boost::system::error_code ec, const Endpoint& endpoint);
  size_t write_handler(boost::system::error_code ec, size_t transferred);
  void read_handler(boost::system::error_code ec, size_t transferred);
  const std::string& get_response() const noexcept;
  const std::string& get_request() const noexcept;

private:
  boost::asio::ip::tcp::resolver resolver;
  boost::asio::ip::tcp::socket socket;
  std::string request, response;
  const std::string host;
};

main() generates Requests to example.com and handles dealing with asynchronous tasks:

int main() {
  size_t n_requests{ 1 }, n_threads{ 1 };
  boost::asio::io_context io_context{ static_cast<int>(n_threads) };

  std::vector<Request> requests;
  std::generate_n(std::back_inserter(requests), n_requests, [&io_context] {
    return Request{ io_context, "example.com" };
  });

  std::cout << "debug:\n " << requests[0].get_request() << std::endl;

  std::vector<std::future<void>> futures;

  for (auto i=0; i!=n_requests; ++i) {
    futures.emplace_back(std::async(std::launch::async, [&io_context] { io_context.run(); }));
  }

  for(auto& future : futures)
    future.get();
  for(const auto& request : requests)
    std::cout << "Response had length " << request.get_response().size();
}

Here is the Godbolt link to the whole program.

As it can be seen, the request generates error output on godbolt. On my station, sometimes the request simply hangs, sometimes it immediately exits with a Segmentation fault error.

This happens no matter the number > 0 of threads (n_threads) or requests (n_requests) generated.

I suspect the problem lying with the fact that, going by the book,std::futures cannot be copied, only moved.

Inside the code, construction of the std::vector<std::future> container for these futures with std::vector::emplace_back() might fail to abide to that constraint.


Solution

  • Your Request isn't properly move-constructible which is required for

    std::generate_n(std::back_inserter(requests), n_requests, [&io_context] {
        return Request{io_context, "example.com"};
    });
    

    to work. When the Request is created, it does

    resolver.async_resolve(
        this->host, "http",
        [this](boost::system::error_code ec, const ResolveResult& results) {
    //   ^^^^
            resolution_handler(ec, results);
        });
    

    which captures the temporary Request. This temporary Request is destroyed when the push_back by the back_inserter into the vector is done. When the resolution_handler is later called on this Request, it will be gone (the this pointer is dangling) and you have undefined behavior.

    A temporary fix to this is to not move Requests into the vector<Request> but to emplace_back them:

    std::vector<Request> requests;
    requests.reserve(n_requests);
    for(size_t i = 0; i != n_requests; ++i) {
        requests.emplace_back(io_context, "example.com");
    }
    

    I would also delete the move constructor to not accidentally run into this problem again until you've figured out how to make Request properly moveable (if that's even wanted). If you do delete the move constructor, you can store Requests in a std::list<Request> instead:

    std::list<Request> requests;
    for (size_t i = 0; i != n_requests; ++i) {
        requests.emplace_back(io_context, "example.com");
    }
    std::cout << "debug:\n" << requests.front().get_request() << std::endl;