Multithreading and internal router

[UltraCoderRU]

Hello. I use internal request router in my project, and it works fine. But since I tried to add multithreading, I regularly get error: event_handler error: connection not found.
And sometimes I got SEGFAULT in via::comms::connection constructor:

...
rx_buffer_(new Container(rx_buffer_size_, 0))
...

Here is my code snippet:

using http_server_type = via::http_server<via::comms::tcp_adaptor, std::string, true>;
using MethodHandler = std::function<void(const HttpRequest&, HttpResponse&)>;

class RestServer {
...
  std::unique_ptr<http_server_type> server_;
  std::vector<std::thread> workers_;
  ASIO::io_service io_;
};

RestServer::RestServer(uint16_t port, uint8_t maxThreads)
{
  server_.reset(new http_server_type(io_));
  route("/resource1", resource1_);
  route("/resource2", resource2_);
  ...
  start(port, maxThreads);
}

void RestServer::routeMethod(const std::string& path,
  via::http::request_method::id method,
  MethodHandler handler)
{
  auto& router = server_->request_router();
  router.add_method(method, path, [handler]
    (const via::http::rx_request& req,
     const via::http::Parameters& params,
     const std::string& data,
     std::string& response_body)
    {
       HttpRequest request(params, data);
       HttpResponse response;

       // Call my handler
       handler(request, response);

       via::http::tx_response res(static_cast<via::http::response_status::code>(response.getStatus()));
       res.add_header(via::http::header_field::id::CONTENT_TYPE, response.getContentType());
       response_body = std::move(response.getBody());
       return res;
  });
}

void RestServer::route(const std::string& path, HttpResource& resource)
{
  using namespace via::http::request_method;
  using namespace std::placeholders;
  routeMethod(path, GET, std::bind(&HttpResource::onGet, &resource, _1, _2));
  routeMethod(path, POST, std::bind(&HttpResource::onPost, &resource, _1, _2));
  routeMethod(path, PUT, std::bind(&HttpResource::onPut, &resource, _1, _2));
  routeMethod(path, DELETE, std::bind(&HttpResource::onDelete, &resource, _1, _2));
}

void RestServer::start(uint16_t port, uint8_t maxThreads)
{
  ASIO_ERROR_CODE error(server_->accept_connections(port));
  if (error)
  {
    LOG_ERROR << error.message();
    throw std::runtime_error("Can not start HTTP server");
  }

  for (uint8_t i = 0; i < maxThreads; i++)
    workers_.emplace_back([this] { io_.run(); });
}

So, is it possible to use internal router in multi-threaded server?
Best regards.

[kenba]

Hello Kirill, thank you for bringing this to my attention.

I've had a look through your code snippet and it looks fine to me. What is especially fine is the first line:

using http_server_type = via::http_server<via::comms::tcp_adaptor, std::string, true>;

Indicating that your http_server_type is configured with use_strand = true , i.e. in `multi-threading" mode.

So to answer your question: yes it should be possible to use the internal router in a multi-threaded server. This is clearly a bug in via::http_server.

I'm busy on another project at the moment, but I'll try to fix this issue ASAP. In the meantime I'd appreciate any further information that you can find about the issue.

Best regards
Ken

[kenba]

Hello Kirill,

I've changed the event and error callback function types to take a shared pointer instead of a weak pointer. The connection pointer is "owned" by the connection class whilst the internal router event handler is called, which should protect against an asynchronous disconnect, which is my best guess for the cause of this issue.

Unfortunately I can't recreate the issue myself. I've checked in the changes into the develop branch. Please can you test the changes and let me know if it's any better now?

Best regards
Ken

[UltraCoderRU]

Hello, Ken.
Unfortunately, my problem still occurs. I created test project, which you can use to reproduce the issue.
Some instructions:

1. Build project with option HTTP_THREAD_SAFE=ON (default).
2. Run: via_http_sample <port> <threads>. Use threads > 1.
3. While application is running, use Apache Benchmark to make some requests:
   ab -n 10000 http://localhost:8080/sample
   You can add -c 2 option to check concurrent requests, but I see this problem even without concurrent requests.

Best regards,
Kirill.

[kenba]

Thank you Kirill,

I was able to recreate the issue by running Apache Benchmark on a separate machine against the example multi-threaded HTTP server: thread_pool_http_server.cpp .
The issue was caused by asynchronous connects, and disconnects.

The server now runs OK against the following ab commands:

ab -n 10000 -c 1000 192.168.1.64:80/hello

ab -n 10000 -c 1000 -k 192.168.1.64:80/hello    

Please let me know if this has fixed the issue on your system.

Thanks again
Ken

[UltraCoderRU]

Ken,
thanks, there is no SEGFAULT now, but I still get error message:
http_server, event_handler error: connection not found
Even in thread_pool_http_server example:

# ./thread_pool_http_server 8080 | grep error
http_server, event_handler error: connection not found
http_server, event_handler error: connection not found
...

Best regards,
Kirill.

[kenba]

Hello Krill,
Sorry about that. I've taken the time to look at a video by Chris Kohlhoff on youtube and I've now got a better understanding of the issue.

I've reverted the previous changes. In particular: I've changed the callbacks back to taking weak pointers again and added some more strand wraps in the connection class to ensure that the connection event handlers in the servers are called in the correct order.

I've tested it with both the thread_pool_http_server and your test project and they both work OK without the error message.

Please let me know if this has fixed the issue on your system.

Best regards
Ken

[UltraCoderRU]

Hello, Ken.

I don't see any errors too. Thank you for help.

Best regards,
Kirill.

[kenba]

Hello Kirill,

Just to let you know that I've modified this change for single threaded mode (see issue #12) and it's now in the main branch, tag: 1.3.1.

I'm currently working on a further change to use thread safe containers (see issue #13). I want to remove the use_strand template parameter and use a new HTTP_THREAD_SAFE macro instead (which was one of your macros).

I want to make you aware of it, since this change will break the interface. If you'd like to try it out, the change to the macro from use_strand is in the develop branch.

Best regards
Ken