Re: [Boost-users] [boost.asio] Concurrently Ping 1000+ Hosts

Subject: Re: [Boost-users] [boost.asio] Concurrently Ping 1000+ Hosts
From: Gavin Lambert (gavinl_at_[hidden])
Date: 2013-12-18 20:24:53

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.