Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from boost/thread/future.hpp to C++11 future. #841

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

igorpeshansky
Copy link

@igorpeshansky igorpeshansky commented May 25, 2018

This fixes bugs with rethrown exceptions (e.g., avoids std::exception_ptr being thrown).

Should fix #776 and #872.

@igorpeshansky
Copy link
Author

@davidbtucker FYI.

@deanberris deanberris self-requested a review May 26, 2018 13:22
Copy link
Member

@deanberris deanberris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @igorpeshansky -- I'm going to need a little time to review this properly. Can you provide a bit more context?

We've encountered problems in the past with the 'ready(...)' accessor which is why we've moved back to using the Boost.Future implementation which allows for checking whether a future is ready without waiting.

@igorpeshansky
Copy link
Author

@deanberris Thanks for taking a look!

The problem is that boost::future has a bug in handling std::exception_ptr (there is a specialization of set_exception for boost::exception_ptr, but not for std::exception_ptr, so it ends up calling boost::copy_exception on an instance of std::exception_ptr). The following code demonstrates the issue:

#include <boost/network/protocol/http/client.hpp>
#include <boost/network/protocol/http/server.hpp>
#include <iostream>
#include <thread>

namespace http = boost::network::http;

class Server {
  struct Handler;
  using http_server = http::server<Handler>;
 public:
  Server()
      : handler_(),
        server_(http_server::options(handler_).address("127.0.0.1").port("")) {
    server_.listen();
    http_server& server = server_;
    server_thread_ = std::thread([&server] { server.run(); });
  }
  ~Server() {
    server_.stop();
    server_thread_.join();
  }
  std::string port() { return server_.port(); }

 private:
  struct Handler {
    void operator()(http_server::request const &request,
                    http_server::connection_ptr connection) {
      connection->set_status(http_server::connection::ok);
      //connection->set_headers(std::map<std::string, std::string>({
      //    {"Content-Type", "text/plain"},
      //}));
      connection->write("");
    }
  };

  Handler handler_;
  http_server server_;
  std::thread server_thread_;
};

int main(int ac, char** av) {
  // Start a server in a separate thread, and allow it to choose an
  // available port.
  Server server;

  http::client client;
  http::client::request request(
    std::string("http://127.0.0.1:") + server.port());
  try {
    try {
      http::client::response response = client.get(request);
      if (status(response) < 300) {
        std::cerr << "Success: " << body(response) << std::endl;
      } else {
        std::cerr << "Failure: " << status(response) << " "
                  << status_message(response) << std::endl;
      }
    } catch (const std::exception_ptr& eptr) {
      std::cerr << "Huh? An exception_ptr?" << std::endl;
      std::rethrow_exception(eptr);
    }
  } catch (const boost::system::system_error& e) {
    std::cerr << "Boost Exception: " << e.what() << std::endl;
  } catch (const std::exception& e) {
    std::cerr << "Exception: " << e.what() << std::endl;
  }
}

Without this change, the output is:

Huh? An exception_ptr?
Exception: Invalid Version Part.

With this change, it's just:

Exception: Invalid Version Part.

An alternative could be to add a specialization for boost::future::set_exception(std::exception_ptr), but that seems like a much more involved option...

@deanberris
Copy link
Member

Thanks for explaining @igorpeshansky -- there were changes in 0.13-release which addresses some of the internals of the HTTP connection handling which has to do with exception handling/propagation. I'm wondering whether we should be attempting to cherry-pick that (or port it) into master -- and whether that's the correct fix for this particular problem.

I actually don't see where, in your example, how we're throwing exceptions at all. Is there something else I'm missing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

boost::exception_detail::clone_impl thrown with 0.13-release
2 participants