2

I am trying to implement some keep-alive service in UDP using BOOST::ASIO, these are the general steps:

  1. Sending keep-alives to 2 processes on the same machine, they are listening on the same ip with a different port.

  2. Loop to send async_send_to to both, and the callback is a function that calls async_receive_from with a callback F(). Both refer to the same endpoint and data buffers.

  3. while loop with io_service.run_one() inside.

The processes reply immediately.

The issue is that sporadically I either get the 2 differing ports when I check the endpoints' ports (the wanted case) F() runs, or, I get twice the same port.

It seems as the endpoint buffer (and probably the data) is getting overwritten by the later packet.

I was thinking the since I'm using run_one() the packets should be processed one by one and there will be no overwriting.

Initial send -

        void GetInstancesHeartbeat(udp::endpoint &sender_endpoint)
        {
            int instanceIndex = 0;
            for (; instanceIndex <= amountOfInstances ; instanceIndex++)
            {
                udp::endpoint endpoint = udp::endpoint(IP, Port+ instanceIndex);
                m_instancesSocket->async_send_to(
                      boost::asio::buffer((char*)&(message),
                      sizeof(message)),endpoint,
                      boost::bind(&ClusterManager::handle_send_to_instance, 
                      this, boost::asio::placeholders::error,
                      boost::asio::placeholders::bytes_transferred,
                      sender_endpoint));
            }
        }

Then the handler -

        void handle_send_to_instance(const boost::system::error_code& error, size_t 
                                         bytes_recvd, udp::endpoint &sender_endpoint)
        {
            m_instancesSocket->async_receive_from(
                boost::asio::buffer(m_dataBuffer, m_maxLength), m_endpoint,
                boost::bind(&ClusterManager::handle_receive_from_instance, this,
                boost::asio::placeholders::error,
                boost::asio::placeholders::bytes_transferred,
                sender_endpoint));
        }

While loop -

        while(true){
            io_service.run_one();
        }

And the handle receive where the port results twice the same -

        void handle_receive_from_instance(const boost::system::error_code& error, size_t 
                                             bytes_recvd, udp::endpoint&sender_endpoint)
            {
                if (!error && bytes_recvd > 0)
                {
                    int instancePort = m_endpoint.port();
                } else {
                    //PRINT ERROR
                }
            }
sehe
  • 374,641
  • 47
  • 450
  • 633
astrachan
  • 61
  • 1

1 Answers1

0

The actual operations are asynchronous, so there's no telling when the endpoint reference gets written to. That's the nature of asynchronous calls.

So, what you need to have is an endpoint receiving variable per asynchronous call (you might store it per instance index).

There are a number of other really suspicious bits:

  • what's the type of message? For most types you'd write just boost::asio::buffer(message) (which deals with T [], std::vector<T>, array<T> etc). This works when T is char or any POD type.

    If message is actually a struct of some type, consider using a single-element array to avoid having to dangerous casting:

    Live On Coliru

    POD message[1] = {pod};
    
    s.async_send_to(boost::asio::buffer(message), udp::endpoint{{}, 6767}, [](boost::system::error_code ec, size_t transferred) {
            std::cout << "Transferred: " << transferred << " (" << ec.message() << ")\n";
        });
    

    (Sends 12 bytes on a typical system).

    Whatever you do, don't write the unsafe C-style cast (Why use static_cast<int>(x) instead of (int)x?).

  • You have while(true) { io.run_one(); } which is an infinite loop. A better way to write it would be: while(io.run_one()) {}

  • However, that would basically be the same as io.run();, but less correctly and less efficiently (see https://www.boost.org/doc/libs/1_68_0/boost/asio/detail/impl/scheduler.ipp line 138), so why not use it?

sehe
  • 374,641
  • 47
  • 450
  • 633
  • The message is a struct, I don't get any issues with this casting but I'll look into it. I considered the solution with an array of endpoint, but as far as I understand, although I'm calling the handle_receive_from_instance by a corresponding async_send_to from a specific instance (and unique port) when the async_receive_from is called I can't know if it actually came from the specific instance that initiated that async before. Correct me if I'm wrong but the first instance to reply will be simply picked up by any async_receive_from. I could pass the id as a parameter if the handlers synced.. – astrachan Dec 07 '18 at 08:48
  • Yeah you'd need both. It's pretty common to have a connection type that encapsulates all state related to an endpoint, so all handlers can bombs to that. Even though UDP is connection-less you could use the same idea here – sehe Dec 07 '18 at 08:52
  • 1
    I think I understood how it can be done using an array of endpoints. If I write to an array of endpoint and data buffers and pass the index to the handler then I check the port of the endpoint on the returned index then I should have it. I'll try that, thanks! – astrachan Dec 07 '18 at 08:58