
On 19/12/2013 13:40, Quoth Kyle Ketterer:
Since I will now be using one io_service object, how can I stop the threads from blocking? When I had an io_service per object, I could just call io_service.stop() and I could successfully call a join() on the thread. Since I am now passing a reference to an io_service object, it seems as though the thread will not join().
You only stop() the io_service once all the pings are done. Or if the PingX object internally knows when it's done (eg. if it reads the right things, or times out) then you just let them not requeue their async_* work when they're done and they'll "fall out" automatically. If you don't have an explicit io_service::work object then the run() will automatically terminate once there are no outstanding async_* jobs or in-progress handler calls; then you don't need to stop() at all unless the user wants to cancel before all the pings have finished. (And even then, you can just cancel the pings instead.)
icmp::resolver::query query(icmp::v4(), ip_addr, ""); destination_ = *resolver_.resolve(query);
You should probably make this async as well, otherwise it will limit performance.
The async functions in start_send and start_receive are strand.wrap() 'd. I call timer.cancel() as well as socket.close() and it seems I can't get it to unblock. Any ideas?
Technically you should cancel/close on the same strand as the read/write operations, as these are not officially cross-thread-safe operations. In practice it usually seems to be safe to not do this though, but it might depend on your platform.
boost::asio::io_service::work work_(io_);
You only need the explicit work object if you are going to have a moment when you're run()ing with no other work (no outstanding async_* requests). Typically this is only an issue if you create the threads first and have some other action that may or may not happen (eg. user activity not involving an incoming network request) that occurs later to initiate the async operations, which does not appear to be the case in your example. In your case, you should just be able to create all your ping objects, which should just queue up an async_resolve (which will then internally queue the async_read/async_send when the resolve completes), so you shouldn't need explicit work.
//buffer etc etc
p.push_back( new XPing ( io_, buffer.data(), boost::ref(ping_results), i) );
Remember that you can't share writable buffers between concurrent workers. If this is constant data that you're sending then this is ok, but otherwise not.
threads.push_back( new boost::thread ( boost::bind(&boost::asio::io_service::run, boost::ref(io_) ) ) );
t_count++;
if(t_count >= max_threads) //join threads after max threads { for(...j...) { threads[j]->join(); }
t_count = 0;
}
}
This is wrong. You should have one loop creating all your ping objects (and setting their initial async work). Then a *separate* loop that creates the threads, and then finally after that a *third* loop that joins them all. And as I said before, your number of threads should not be related to your number of ping objects; it should be related to the # of CPUs, or just a fixed (small) number.