boost::asio doesn't trigger the read handler, while wireshark sees data comming in

Question

I am trying to send some data and act on the reply. I see (using wireshark) that data is sent and received by the system, but boost::asio doesn't trigger my callback. Does somebody has an idea what I am doing wrong?

#include <asio.hpp>
#include <bits/stdint-uintn.h>
#include <chrono>
#include <condition_variable>
#include <cstddef>
#include <iostream>
#include <memory>
#include <mutex>
#include <string>
#include <system_error>
#include <thread>

static const int polynomial = 0x1021; // represents x^16+x^12+x^5+1
uint16_t calc(uint8_t* bytes, std::size_t length)
{
  uint16_t new_crc = 0x0000;

  // bytes part
  for (std::size_t j = 0; j < length; ++j)
  {
    for (int i = 0; i < 8; ++i)
    {
      bool bit = ((bytes[j] >> (7 - i) & 1) == 1);
      bool c15 = ((new_crc >> 15 & 1) == 1);
      new_crc <<= 1;
      // If coefficient of bit and remainder polynomial = 1 xor crc with polynomial
      if (c15 ^ bit) new_crc ^= polynomial;
    }
  }

  return new_crc;
}

int main(int argc, const char* argv[])
{

  asio::io_service main_io_service;

  std::string ip = "192.168.100.155";
  int portP = 4001, portS = 4002;

  auto sPrimary = std::shared_ptr<asio::ip::tcp::socket>(new asio::ip::tcp::socket(main_io_service));
  auto sSecondary = std::shared_ptr<asio::ip::tcp::socket>(new asio::ip::tcp::socket(main_io_service));
  auto epPrimary = asio::ip::tcp::endpoint(asio::ip::address::from_string(ip), portP);
  auto epSecondary = asio::ip::tcp::endpoint(asio::ip::address::from_string(ip), portS);

  std::error_code ec;
  sPrimary->connect(epPrimary, ec);
  if (ec || !sPrimary->is_open())
  {
    std::cerr << "primary failed to connect" << std::endl;
  }

  ec.clear();
  sSecondary->connect(epSecondary, ec);
  if (ec || !sSecondary->is_open())
  {
    std::cerr << "secondary failed to connect" << std::endl;
  }

  std::mutex mutex;
  std::unique_lock<std::mutex> lock(mutex);
  std::condition_variable cv;

  const std::size_t msgSize = 9;
  uint8_t msg[msgSize];
  int i = 0;
  msg[i++] = 0x02;
  msg[i++] = 0xFF;
  msg[i++] = 0x00;
  msg[i++] = 0x00;
  msg[i++] = 0x00;
  msg[i++] = 0x00;

  uint16_t crc = calc(msg, i);

  msg[i++] = (uint8_t) (crc & 0xFF);
  msg[i++] = (uint8_t) (crc >> 8);
  msg[i++] = 0x03;

  const std::size_t buffSize = 1024;
  uint8_t buff[buffSize];
  std::size_t bytesRead = 0;

  asio::async_write((*sPrimary.get()), asio::buffer(msg, msgSize), [&sPrimary, &cv, &buff, &buffSize, &bytesRead](const std::error_code &ec, std::size_t bytesWritten)
  {
    asio::async_read((*sPrimary.get()), asio::buffer(buff, buffSize), [&cv, &bytesRead](const std::error_code &ec, std::size_t currentBytesRead)
    {
      bytesRead += currentBytesRead;
      cv.notify_one();
    });
  });

  main_io_service.run();

  cv.wait(lock);

  for (std::size_t i = 0; i < bytesRead; ++i)
    std::cout << std::hex << buff[i];

  main_io_service.stop();

  return 0;
}

Just added the whole test code that will compile. Although you need a device that answers. This code talks to a serial server that has a piece of hardware that replies on the sent packet.

Thanks!

are you sure your callback is never triggered? Is the data sent back 1024 or more bytes long? If not `async_read` will only return when the socket closes. — Alan Birtles, Dec 11 '19 at 16:13
There's also a chance `notify_one` will be called before `wait` and `wait` will therefore never return — Alan Birtles, Dec 11 '19 at 16:15
`io_service::run` is blocking method. It returns only if all handlers were processed and there are no more outstanding handlers. So the chain of actions in your program is: `handler1` called -> `handler2` called (which invokes `notify_one`) -> `io_service::run` returns -> `wait` is called, and waits forever, because it was not able to catch notification. — rafix07, Dec 11 '19 at 16:24

score 0 · Answer 1 · answered Dec 12 '19 at 19:32

The problem that @rafix07 calls out is your problem.

Even if you "fake it out" by running io_service::run() on another thread, you technically still have a time window for the same race condition.

In general, locking synchronization primitives do not mix with task-based parallelism. In this case it would very much appear you just want to

post another task to the service when the read completes
expire a timer that you can respond to

In the very simple case of your code, you could even use other, simpler, options:

exploit the fact that run() blocks until all handlers completed. That is to say, you can take the sheer fact that run() returned as indication that the read completed:
don't use asynchronous handlers, since it doesn't serve a purpose (this could be down to oversimplified example code here)

4. use synchronous IO

This is by far the simplest. Many other simplifications made throughout the program

Live On Coliru

#include <cstdint>
#include <iostream>
#include <string>

#ifndef NOBOOST
    #include <boost/asio.hpp>
    namespace asio = boost::asio;
    using boost::system::error_code;
#else
    #include <asio.hpp>
    namespace asio = boost::asio;
    using std::error_code;
#endif

static const int polynomial = 0x1021; // represents x^16+x^12+x^5+1
uint16_t calc_crc(uint8_t* bytes, std::size_t length) {
    uint16_t new_crc = 0x0000;

    // bytes part
    for (std::size_t j = 0; j < length; ++j) {
        for (int i = 0; i < 8; ++i) {
            bool bit = ((bytes[j] >> (7 - i) & 1) == 1);
            bool c15 = ((new_crc >> 15 & 1) == 1);
            new_crc <<= 1;
            // If coefficient of bit and remainder polynomial = 1 xor crc with polynomial
            if (c15 ^ bit)
                new_crc ^= polynomial;
        }
    }

    return new_crc;
}

int main() {
    static const std::string ip = "127.0.0.1";
    static const unsigned short portP = 4001, portS = 4002;

    using asio::ip::address;

    asio::io_service io;
    asio::ip::tcp::socket sPrimary(io), sSecondary(io);

    sPrimary.connect({ address::from_string(ip), portP });
    sSecondary.connect({ address::from_string(ip), portS });

    uint8_t msg[] {
        0x02, 0xFF, 0x00, 0x00, 0x00, 0x00,
        0x00, 0x00, //crc
        0x03
    };

    {   // set crc
        uint16_t const crc = calc_crc(msg, sizeof(msg)-3);
        msg[sizeof(msg)-3] = (uint8_t)(crc & 0xFF);
        msg[sizeof(msg)-2] = (uint8_t)(crc >> 8);
    }

    std::string buff;

    auto bytesWritten = asio::write(sPrimary, asio::buffer(msg));
    std::cout << bytesWritten << " sent" << std::endl;
    auto bytesRead    = asio::read(sPrimary, asio::dynamic_buffer(buff), asio::transfer_exactly(32));
    std::cout << bytesRead << " received" << std::endl;

    for (uint8_t ch : buff)
        std::cout << std::hex << static_cast<int>(ch);
    std::cout << std::endl;
}

Prints

9 sent
32 received
23696e636c756465203c63737464696e743ea23696e636c756465203c696f73

And indeed, that's the hex encoding of the first 32 bytes of main.cpp

3. use implicit completion

Trust that the handlers ran if run() returns (error handling would be required). The code is essentially the same but with more elaborate concerns around lambda captures and object lifetimes.

Note: all other simplifications still apply

Live On Coliru

asio::async_write(sPrimary, asio::buffer(msg), [&sPrimary, &buff](error_code ec, size_t bytesWritten) {
    std::cout << "async_write: " << ec.message() << ", " << bytesWritten << " sent" << std::endl;
    asio::async_read(sPrimary, asio::dynamic_buffer(buff), asio::transfer_exactly(32), [](error_code ec, size_t bytesRead) {
        std::cout << "async_read: " << ec.message() << ", " << bytesRead << " received" << std::endl;
    });
});

io.run();

for (uint8_t ch : buff)
    std::cout << std::hex << static_cast<int>(ch);
std::cout << std::endl;

Prints:

async_write: Success, 9 sent
async_read: Success, 32 received
23696e636c756465203c63737464696e743ea23696e636c756465203c696f73

2. use a timer signal

This most closely "resembles" the CV approach you had, by using a timer object to represent the condition.

notably, this does error handling better than the above "3." code
note also, it guarantees to call the completion handler of signal_complete (unless the program terminates prematurely)
as such, the information is in the expiry() of the timer, not in the error code (the time will always appear canceled)

Live On Coliru

std::string buff;
asio::high_resolution_timer signal_complete(io, std::chrono::high_resolution_clock::time_point::max());
signal_complete.async_wait([&signal_complete, &buff](error_code ec) {
     std::cout << "signal_complete: " << ec.message() << std::endl;

     if (signal_complete.expiry() < std::chrono::high_resolution_clock::now()) {
        for (uint8_t ch : buff)
            std::cout << std::hex << static_cast<int>(ch);
        std::cout << std::endl;
     }
});

asio::async_write(sPrimary, asio::buffer(msg), [&sPrimary, &buff, &signal_complete](error_code ec, size_t bytesWritten) {
    std::cout << "async_write: " << ec.message() << ", " << bytesWritten << " sent" << std::endl;
    asio::async_read(sPrimary, asio::dynamic_buffer(buff), asio::transfer_exactly(32), [&signal_complete](error_code ec, size_t bytesRead) {
        std::cout << "async_read: " << ec.message() << ", " << bytesRead << " received" << std::endl;

        if (!ec) {
            signal_complete.expires_at(std::chrono::high_resolution_clock::time_point::min());
        } else {
            signal_complete.cancel();
        }
    });
});

io.run();

Prints:

async_write: Success, 9 sent
async_read: Success, 32 received
signal_complete: Operation canceled
23696e636c756465203c63737464696e743ea23696e636c756465203c696f73

1. Post another task when read completes

This is the most natural fit to most async IO scenarios, because it puts all tasks in the same queue.

The only part that is further complicated is getting the life-times of (shared) objects right.

Live On Coliru

std::string buff;

asio::async_write(sPrimary, asio::buffer(msg), [&io, &sPrimary, &buff](error_code ec, size_t bytesWritten) {
    std::cout << "async_write: " << ec.message() << ", " << bytesWritten << " sent" << std::endl;
    asio::async_read(sPrimary, asio::dynamic_buffer(buff), asio::transfer_exactly(32), [&io, &buff](error_code ec, size_t bytesRead) {
        std::cout << "async_read: " << ec.message() << ", " << bytesRead << " received" << std::endl;

        if (!ec) {
            post(io, [&buff] {
                for (uint8_t ch : buff)
                    std::cout << std::hex << static_cast<int>(ch);
                std::cout << std::endl;
            });
        }
    });
});

io.run();

Printing, again:

async_write: Success, 9 sent
async_read: Success, 32 received
23696e636c756465203c63737464696e743ea23696e636c756465203c696f73

boost::asio doesn't trigger the read handler, while wireshark sees data comming in

1 Answers1

4. use synchronous IO

3. use implicit completion

2. use a timer signal

1. Post another task when read completes