From 2307fc0841f5afa43413b09ce8b3c901535c5422 Mon Sep 17 00:00:00 2001
From: Corentin Le Molgat <corentinl@google.com>
Date: Fri, 10 Nov 2023 14:10:49 +0100
Subject: [PATCH] constraint_solver: prevent search operator infinite loop

---
 ortools/constraint_solver/routing.cc          |  6 +++++-
 .../routing_neighborhoods.cc                  | 21 +++++++++++++------
 .../constraint_solver/routing_neighborhoods.h |  6 ++++--
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/ortools/constraint_solver/routing.cc b/ortools/constraint_solver/routing.cc
index 45e6bfcc9e8..65081ced0fe 100644
--- a/ortools/constraint_solver/routing.cc
+++ b/ortools/constraint_solver/routing.cc
@@ -3970,7 +3970,11 @@ void RoutingModel::CreateNeighborhoodOperators(
       CreateCPOperator<ExtendedSwapActiveOperator>();
   std::vector<std::vector<int64_t>> alternative_sets(disjunctions_.size());
   for (const RoutingModel::Disjunction& disjunction : disjunctions_) {
-    alternative_sets.push_back(disjunction.indices);
+    // Only add disjunctions of cardinality 1, as
+    // SwapActiveToShortestPathOperator only supports DAGs.
+    if (disjunction.value.max_cardinality == 1) {
+      alternative_sets.push_back(disjunction.indices);
+    }
   }
   local_search_operators_[SHORTEST_PATH_SWAP_ACTIVE] =
       CreateOperator<SwapActiveToShortestPathOperator>(
diff --git a/ortools/constraint_solver/routing_neighborhoods.cc b/ortools/constraint_solver/routing_neighborhoods.cc
index e1fb01a9ec2..e630ab32033 100644
--- a/ortools/constraint_solver/routing_neighborhoods.cc
+++ b/ortools/constraint_solver/routing_neighborhoods.cc
@@ -130,7 +130,8 @@ SwapActiveToShortestPathOperator::SwapActiveToShortestPathOperator(
       arc_evaluator_(std::move(arc_evaluator)),
       alternative_sets_(std::move(alternative_sets)),
       to_alternative_set_(vars.size(), -1),
-      path_predecessor_(vars.size(), -1) {
+      path_predecessor_(vars.size(), -1),
+      touched_(vars.size()) {
   for (int i = 0; i < alternative_sets_.size(); ++i) {
     for (int j : alternative_sets_[i]) {
       if (j < to_alternative_set_.size()) to_alternative_set_[j] = i;
@@ -149,10 +150,10 @@ bool SwapActiveToShortestPathOperator::MakeNeighbor() {
     next = Next(next);
   }
   if (alternatives.empty()) return false;
+  const int sink = next;
   next = OldNext(before_chain);
   bool swap_done = false;
-  UpdateShortestPath(before_chain, next, alternatives);
-  for (int64_t node : path_) {
+  for (int64_t node : GetShortestPath(before_chain, sink, alternatives)) {
     if (node != next) {
       SwapActiveAndInactive(next, node);
       swap_done = true;
@@ -162,10 +163,10 @@ bool SwapActiveToShortestPathOperator::MakeNeighbor() {
   return swap_done;
 }
 
-void SwapActiveToShortestPathOperator::UpdateShortestPath(
+const std::vector<int64_t>& SwapActiveToShortestPathOperator::GetShortestPath(
     int source, int sink, const std::vector<int>& alternative_chain) {
   path_.clear();
-  if (alternative_chain.empty()) return;
+  if (alternative_chain.empty()) return path_;
   // Initializing values at the first "layer" after the source (from source to
   // all alternatives at rank 0).
   const std::vector<int64_t>& first_alternative_set =
@@ -219,12 +220,20 @@ void SwapActiveToShortestPathOperator::UpdateShortestPath(
       predecessor = last_alternative_set[alternative];
     }
   }
-  if (predecessor == -1) return;
+  if (predecessor == -1) return path_;
   // Build the path from predecessors on the shortest path.
   path_.resize(alternative_chain.size(), predecessor);
+  touched_.SparseClearAll();
+  touched_.Set(predecessor);
   for (int rank = alternative_chain.size() - 2; rank >= 0; --rank) {
     path_[rank] = path_predecessor_[path_[rank + 1]];
+    if (touched_[path_[rank]]) {
+      path_.clear();
+      return path_;
+    }
+    touched_.Set(path_[rank]);
   }
+  return path_;
 }
 
 MakePairActiveOperator::MakePairActiveOperator(
diff --git a/ortools/constraint_solver/routing_neighborhoods.h b/ortools/constraint_solver/routing_neighborhoods.h
index 6955aed8ae8..ad3458cadf4 100644
--- a/ortools/constraint_solver/routing_neighborhoods.h
+++ b/ortools/constraint_solver/routing_neighborhoods.h
@@ -24,6 +24,7 @@
 #include "ortools/constraint_solver/constraint_solver.h"
 #include "ortools/constraint_solver/constraint_solveri.h"
 #include "ortools/constraint_solver/routing_types.h"
+#include "ortools/util/bitset.h"
 
 namespace operations_research {
 
@@ -124,14 +125,15 @@ class SwapActiveToShortestPathOperator : public PathOperator {
   }
 
  private:
-  void UpdateShortestPath(int source, int sink,
-                          const std::vector<int>& alternative_chain);
+  const std::vector<int64_t>& GetShortestPath(
+      int source, int sink, const std::vector<int>& alternative_chain);
 
   RoutingTransitCallback2 arc_evaluator_;
   const std::vector<std::vector<int64_t>> alternative_sets_;
   std::vector<int> to_alternative_set_;
   std::vector<int64_t> path_predecessor_;
   std::vector<int64_t> path_;
+  SparseBitset<int64_t> touched_;
 };
 
 /// Pair-based neighborhood operators, designed to move nodes by pairs (pairs