The Wayback Machine - https://web.archive.org/web/20210112002704/https://github.com/cpp-netlib/cpp-netlib/pull/841
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: master
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

@igorpeshansky igorpeshansky commented May 25, 2018

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

@deanberris deanberris left a comment

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

@igorpeshansky igorpeshansky commented May 31, 2018

@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

@deanberris deanberris commented Jun 13, 2018

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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.