Fix segfault sorting runnable steps

Same problem as d744362e4a.

    at /nix/store/ksvsbr7pg4z69bv6fbbc8h7x7rm2104m-gcc-4.9.3/include/c++/4.9.3/bits/predefined_ops.h:166
    __last@entry=..., __comp=...) at /nix/store/ksvsbr7pg4z69bv6fbbc8h7x7rm2104m-gcc-4.9.3/include/c++/4.9.3/bits/stl_algo.h:1827
    __comp=...) at /nix/store/ksvsbr7pg4z69bv6fbbc8h7x7rm2104m-gcc-4.9.3/include/c++/4.9.3/bits/stl_algo.h:4717

src/hydra-queue-runner/dispatcher.cc

@@ -144,13 +144,40 @@ system_time State::doDispatch()
 
            FIXME: O(n lg n); obviously, it would be better to keep a
            runnable queue sorted by priority. */
-        std::vector<Step::ptr> runnableSorted;
+        struct StepInfo
+        {
+            Step::ptr step;
+
+            /* The lowest share used of any jobset depending on this
+               step. */
+            double lowestShareUsed = 1e9;
+
+            /* Info copied from step->state to ensure that the
+               comparator is a partial ordering (see MachineInfo). */
+            int highestGlobalPriority;
+            int highestLocalPriority;
+            BuildID lowestBuildID;
+
+            StepInfo(Step::ptr step, Step::State & step_) : step(step)
+            {
+                for (auto & jobset : step_.jobsets)
+                    lowestShareUsed = std::min(lowestShareUsed, jobset->shareUsed());
+                highestGlobalPriority = step_.highestGlobalPriority;
+                highestLocalPriority = step_.highestLocalPriority;
+                lowestBuildID = step_.lowestBuildID;
+            }
+        };
+
+        std::vector<StepInfo> runnableSorted;
+
         struct RunnablePerType
         {
             unsigned int count{0};
             std::chrono::seconds waitTime{0};
         };
+
         std::unordered_map<std::string, RunnablePerType> runnablePerType;
+
         {
             auto runnable_(runnable.lock());
             runnableSorted.reserve(runnable_->size());
@@ -170,37 +197,26 @@ system_time State::doDispatch()
 
                 /* Skip previously failed steps that aren't ready
                    to be retried. */
-                {
+                auto step_(step->state.lock());
-                    auto step_(step->state.lock());
+                r.waitTime += std::chrono::duration_cast<std::chrono::seconds>(now - step_->runnableSince);
-                    r.waitTime += std::chrono::duration_cast<std::chrono::seconds>(now - step_->runnableSince);
+                if (step_->tries > 0 && step_->after > now) {
-                    if (step_->tries > 0 && step_->after > now) {
+                    if (step_->after < sleepUntil)
-                        if (step_->after < sleepUntil)
+                        sleepUntil = step_->after;
-                            sleepUntil = step_->after;
+                    continue;
-                        continue;
-                    }
                 }
 
-                runnableSorted.push_back(step);
+                runnableSorted.emplace_back(step, *step_);
             }
         }
 
-        for (auto & step : runnableSorted) {
-            auto step_(step->state.lock());
-            step_->lowestShareUsed = 1e9;
-            for (auto & jobset : step_->jobsets)
-                step_->lowestShareUsed = std::min(step_->lowestShareUsed, jobset->shareUsed());
-        }
-
         sort(runnableSorted.begin(), runnableSorted.end(),
-            [](const Step::ptr & a, const Step::ptr & b)
+            [](const StepInfo & a, const StepInfo & b)
             {
-                auto a_(a->state.lock());
-                auto b_(b->state.lock()); // FIXME: deadlock?
                 return
-                    a_->highestGlobalPriority != b_->highestGlobalPriority ? a_->highestGlobalPriority > b_->highestGlobalPriority :
+                    a.highestGlobalPriority != b.highestGlobalPriority ? a.highestGlobalPriority > b.highestGlobalPriority :
-                    a_->lowestShareUsed != b_->lowestShareUsed ? a_->lowestShareUsed < b_->lowestShareUsed :
+                    a.lowestShareUsed != b.lowestShareUsed ? a.lowestShareUsed < b.lowestShareUsed :
-                    a_->highestLocalPriority != b_->highestLocalPriority ? a_->highestLocalPriority > b_->highestLocalPriority :
+                    a.highestLocalPriority != b.highestLocalPriority ? a.highestLocalPriority > b.highestLocalPriority :
-                    a_->lowestBuildID < b_->lowestBuildID;
+                    a.lowestBuildID < b.lowestBuildID;
             });
 
         /* Find a machine with a free slot and find a step to run
@@ -211,7 +227,8 @@ system_time State::doDispatch()
         for (auto & mi : machinesSorted) {
             if (mi.machine->state->currentJobs >= mi.machine->maxJobs) continue;
 
-            for (auto & step : runnableSorted) {
+            for (auto & stepInfo : runnableSorted) {
+                auto & step(stepInfo.step);
 
                 /* Can this machine do this step? */
                 if (!mi.machine->supportsStep(step)) continue;

src/hydra-queue-runner/state.hh

@@ -171,10 +171,6 @@ struct Step
            step. */
         int highestGlobalPriority{0};
 
-        /* The lowest share used of any jobset depending on this
-           step. */
-        double lowestShareUsed;
-
         /* The highest local priority of any build depending on this
            step. */
         int highestLocalPriority{0};