icecc / icecream

Distributed compiler with a central scheduler to share build load
GNU General Public License v2.0
1.61k stars 252 forks source link

Bugged timeout in DiscoverSched::getNetnames #609

Open Zalewa opened 1 year ago

Zalewa commented 1 year ago

There are several issues with the timeout argument in DiscoverSched::getNetnames:

  1. Mixed units. The docstring and the usage indicates that the timeout is expressed in milliseconds, but then its compared in min() against seconds
        /* Wait at least two seconds to give all schedulers a chance to answer
           (unless that'd be longer than the timeout).*/
        time_t timeout_time = time(nullptr) + min(2 + 1, timeout); // <-- timeout is in ms, time() is in seconds
  2. The timeout of the do/while loop is based on a wall clock instead of a monotonic clock. It's susceptible to wall clock changes (ntpdate).
  3. The granularity of the do/while loop has a 1 second precision because it's based on time(). The timeout is an integer, divided by 1000. The only case where this method is called, it's called with timeout 200, meaning that the timeout is effectively 0 seconds. This means that this do/while loop always runs exactly once.