Hi there,

AFAIK, you need an accepted socket for the async_accept call and you need
to create a new socket every time when you call async_accept and this
socket will be filled with an actual descriptor when a new connection is
accepted.
I personally, find another version of `async_accept` more user friendly -
https://www.boost.org/doc/libs/1_84_0/doc/html/boost_asio/reference/basic_socket_acceptor/async_accept/overload3.html
This overload doesn't require you to provide the socket - it just gives the
newly accepted socket to you.
Then of course you need to `std::move` the accepted socket if you want to
keep it alive in a member variable because it's a move-only resource.
Or you can use coroutines and let the coroutine frame keep the needed
resources  (socket, request, response) alive.

HTH,
Pavel.

On Thu, Apr 4, 2024 at 4:20 PM Loup VAILLANT via Boost-users <
boost-users@lists.boost.org> wrote:

> Hello,
>
> So this is rather embarrassing. After working more than 2 days full time I
> can't even get the most basic code running. I've scoured the docs and tried
> to find some tutorial, no luck. My onboarding experience so far has been…
> less than perfect.
>
> The reason I'm taking the time to ask here is because I feel there is
> something fundamental, likely obvious to the experts, that I don't
> understand about how beast (and, I'm guessing, asio)  works, and how I'm
> supposed to use it. Which is probably why seemingly innocuous changes to
> official example code are causing memory corruptions left and right.
>
> Let's start at the beginning.
>
> My first attempt with beast was getting this code to work:
>
> https://www.boost.org/doc/libs/1_73_0/libs/beast/example/http/server/small/http_server_small.cpp
>
> Compiling it with `g++ -std=c++20`, gets me a compilation error,
> thankfully easily fixed.
> Then I wait almost 4 seconds for a successful compilation. `-O2` and `-Og
> -g` are worse at 6 seconds.
>
> I know many developers would say a few seconds are no big deal (though wen
> we get to XKCD sword fights it kind of is), but my problem here is that I
> didn't want to wait every time I made a business logic change to my code.
> But since 95% of compilation time came from including a huge header-only
> library, I figured I'd write a wrapper, that would confine boost/asio/beast
> includes to a single compilation unit. The API I'm aiming for looks like
> this:
>
>     struct Http_message {
>         std::unordered_map<std::string, std::string> headers;
>         std::string                                  body;
>     };
>     struct Http_request : public Http_message {
>         std::string method;
>         std::string target;
>     };
>     struct Http_response : public Http_message {
>         int status;
>     };
>     typedef std::function<Http_response(const Http_request &request)>
> response_cb;
>     void http_start(unsigned short port, response_cb on_respond);
>
> (Note 1: I want web sockets as well, but one problem at a time)
> (Note 2: I tried wrapping around http::request and http::response more
> directly to avoid unnecessary copies, but I ran into lifetime issues.)
>
> To its credit, this small server example seemed to run well enough, and
> could serve as a starting point. Before I untangle the business code from
> the HTTP specific stuff though, I needed to simplify this example a bit.
> So *this* is my actual starting point:
>
>
> #include <boost/beast/core.hpp>
> #include <boost/beast/http.hpp>
>
> namespace beast = boost::beast;
> namespace http  = beast::http;
> namespace net   = boost::asio;
> using tcp       = boost::asio::ip::tcp;
>
> class http_connection : public
> std::enable_shared_from_this<http_connection>
> {
> public:
>     http_connection(tcp::socket socket) : socket_(std::move(socket)){}
>
>     void start()
>     {
>         auto self = shared_from_this();
>         http::async_read(
>             socket_, buffer_, request_,
>             [self](beast::error_code ec, std::size_t) {
>                 if(!ec) { self->process_request(); }
>             });
>         deadline_.async_wait(
>             [self](beast::error_code ec) {
>                 if(!ec) { self->socket_.close(ec); }
>             });
>     }
>
> private:
>     tcp::socket                        socket_;
>     beast::flat_buffer                 buffer_{8192};
>     http::request <http::dynamic_body> request_;
>     http::response<http::dynamic_body> response_;
>     net::steady_timer                  deadline_ {
>         socket_.get_executor(), std::chrono::seconds(60) };
>
>     void process_request() {
>         response_.version(request_.version());
>         response_.keep_alive(false);
>         response_.result(http::status::ok);
>         response_.set(http::field::server, "Beast");
>         response_.set(http::field::content_type, "text/plain");
>         beast::ostream(response_.body())
>             << "Target = " << request_.target()                     << "\n"
>             << "Method = " << std::string(request_.method_string()) <<
> "\n";
>         response_.set(http::field::content_length,
>                       std::to_string(response_.body().size()));
>
>         auto self = shared_from_this();
>         http::async_write(
>             socket_, response_,
>             [self](beast::error_code ec, std::size_t) {
>                 self->socket_.shutdown(tcp::socket::shutdown_send, ec);
>                 self->deadline_.cancel();
>             });
>     }
> };
>
> void http_server(tcp::acceptor& acceptor, tcp::socket& socket)
> {
>     auto cb = [&](beast::error_code ec) {
>         if(!ec)
> {std::make_shared<http_connection>(std::move(socket))->start();}
>         http_server(acceptor, socket);
>     };
>     acceptor.async_accept(socket, cb);
> }
>
> int main(void)
> {
>     auto const      address = net::ip::make_address("0.0.0.0");
>     unsigned short  port    = 8080;
>     net::io_context ioc     {1};
>     tcp::acceptor   acceptor{ioc, {address, port}};
>     tcp::socket     socket  {ioc};
>     http_server(acceptor, socket);
>     ioc.run();
>     return 0;
> }
>
>
> So far it works. It listens to port 8080, and a curl request or 3 to the
> server works as expected:
>
>
> $ curl -XYOLO -v http://localhost:8080/foobar
> *   Trying 127.0.0.1:8080...
> * Connected to localhost (127.0.0.1) port 8080 (#0)
> > YOLO /foobar HTTP/1.1
> > Host: localhost:8080
> > User-Agent: curl/7.81.0
> > Accept: */*
> >
> * Mark bundle as not supporting multiuse
> < HTTP/1.1 200 OK
> < Connection: close
> < Server: Beast
> < Content-Type: text/plain
> < Content-Length: 31
> <
> Target = /foobar
> Method = YOLO
> * Closing connection 0
>
> And yet I can sense a potential problem right off the bat: in the
> `http_server()` function, we call `std::move(socket)`. This move seems to
> be mandatory here, because removing triggers a compilation error. My point
> is, it looks like upon incoming connection we move the socket out of its
> outer context, such that it is no longer bound to the IO context. Is this
> another error in the example?
>
> Okay then, how about I pass the socket all the way by reference instead?
> Let the constructor get a `tcp::socket&`, store this in a `tcp::socket&`
> member variable, remove all the `std::move`. It compiles, it runs the first
> query, but at the second query my program just freezes. Or loops, I don't
> know.
>
> So that's the first thing. I am _really confused_ about how this socket is
> supposed to work. Why does it block when I remove the standard moves? Isn't
> it supposed to be the same socket bound to the same IO context all the way?
> And I don't think it falls out of scope before the program ends either, it
> lives on the stack of the `main()` function, and the `ioc.run()`we call at
> the end blocks until we're done with the async reads (which is, never).
>
> **Can someone explain to me what the hell is going on with this socket?**
>
> Right now though I can accept that some dark secret lies behind this
> `std::move()`, and leave it alone for now.
>
> Moving on, I'll just admit for the time being that something magical and
> forbidden lies between this mysterious `std::move()` and prudently leave it
> behind. Now it's working again, and I can move (no pun intended) to the
> next step: extracting the business logic to an external function. I start
> with a trivial extraction:
>
> void respond(http::response<http::dynamic_body>       &response,
>              const http::request <http::dynamic_body> &request)
> {
>     response.result(http::status::ok);
>     response.set(http::field::content_type, "text/plain");
>     beast::ostream(response.body())
>         << "Target = " << request.target()                     << "\n"
>         << "Method = " << std::string(request.method_string()) << "\n";
> }
>
> And modify `process_request()` accordingly:
>
>     void process_request() {
>         response_.version(request_.version());
>         response_.keep_alive(false);
>         response_.set(http::field::server, "Beast");
>         respond(response_, request_);
>         response_.set(http::field::content_length,
>                       std::to_string(response_.body().size()));
>
>         auto self = shared_from_this();
>         http::async_write(
>             socket_, response_,
>             [self](beast::error_code ec, std::size_t) {
>                 self->socket_.shutdown(tcp::socket::shutdown_send, ec);
>                 self->deadline_.cancel();
>             });
>     }
>
> That works, but this is not quite enough: my goal here is to isolate my
> user code from the overly long compilation times imposed by this huge
> boost::beast. Maybe I can include the request & response headers without
> pulling in the kitchen sink with it, but I don't quite trust it. So let's
> wrap the `http::response` in some class so I can keep my headers nice and
> small:
>
>
> class http_response {
> public:
>     http_response(http::response<http::dynamic_body> &response)
>     : response_(response) {}
>
>     void set_status(int status) {
>         response_.result(status);
>     }
>     void set_header(const std::string &title,
>                     const std::string &value) {
>         response_.set(title, value);
>     }
>     void set_body(const std::string &body) {
>         beast::ostream(response_.body()) << body;
>     }
>
> private:
>     http::response<http::dynamic_body> &response_;
> };
>
>
> Nothing fancy. And with a forward declaration I should be able to avoid
> including the response in the header that defines that class. Now I need to
> modify my business code accordingly:
>
>
> void respond(http_response &response,
>              const http::request<http::dynamic_body> &request)
> {
>     response.set_status(200);
>     response.set_header("Content-Type", "text/plain");
>     response.set_body(
>         "Target = " + std::string(request.target())        + "\n" +
>         "Method = " + std::string(request.method_string()) + "\n");
> }
>
>
> Still not free from `http::request <http::dynamic_body>` yet, but one step
> at a time. And finally, I need to adjust `process_request()`:
>
>     void process_request() {
>         response_.version(request_.version());
>         response_.keep_alive(false);
>         response_.set(http::field::server, "Beast");
>         http_response response(response_);
>         respond(response, request_);
>         response_.set(http::field::content_length,
>                       std::to_string(response_.body().size()));
>
>         auto self = shared_from_this();
>         http::async_write(
>             socket_, response_,
>             [self](beast::error_code ec, std::size_t) {
>                 self->socket_.shutdown(tcp::socket::shutdown_send, ec);
>                 self->deadline_.cancel();
>             });
>     }
>
>
> Huh, what do you know, this time around it worked. I no longer experience
> the lifetime issues I've once had. Okay, I guess I'll add the request
> wrapper, then:
>
>
> class http_request {
> public:
>     http_request(const http::request<http::dynamic_body> &request)
>     : request_(request) {}
>
>     std::string target() const { return std::string(request_.target()); }
>     std::string method() const { return
> std::string(request_.method_string()); }
>
> private:
>     const http::request<http::dynamic_body> &request_;
> };
>
> void respond(http_response &response, const http_request &request)
> {
>     response.set_status(200);
>     response.set_header("Content-Type", "text/plain");
>     response.set_body(
>         "Target = " + request.target() + "\n" +
>         "Method = " + request.method() + "\n");
> }
>
>     void process_request() {
>         response_.version(request_.version());
>         response_.keep_alive(false);
>         response_.set(http::field::server, "Beast");
>         http_response response(response_);
>         http_request  request (request_);
>         respond(response, request);
>         response_.set(http::field::content_length,
>                       std::to_string(response_.body().size()));
>
>         auto self = shared_from_this();
>         http::async_write(
>             socket_, response_,
>             [self](beast::error_code ec, std::size_t) {
>                 self->socket_.shutdown(tcp::socket::shutdown_send, ec);
>                 self->deadline_.cancel();
>             });
>     }
>
>
> And it still works! Maybe it'll work out in the end?
> Regardless, thanks for the rubber ducking I guess.
>
> My first question remains though: what the hell is going on with the
> socket? Why can't I just pass a reference along? Why do we even need a move
> constructor? Is there some special semantics, or `boost::asio` specific
> arcane, I should be privy to?
>
> Loup.
>
> _______________________________________________
> Boost-users mailing list
> Boost-users@lists.boost.org
> https://lists.boost.org/mailman/listinfo.cgi/boost-users
>
_______________________________________________
Boost-users mailing list
Boost-users@lists.boost.org
https://lists.boost.org/mailman/listinfo.cgi/boost-users

Reply via email to