Skip to content

Commit aa06029

Browse files
xlausselxlaussel
committed
References removed for extracted heapNode: could lead to bugs because the same was sometimes modified after when relaxing outgoing edges
1 parent 8697a6b commit aa06029

File tree

6 files changed

+57
-70
lines changed

6 files changed

+57
-70
lines changed

‎include/engine/routing_algorithms/routing_base_ch.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ bool stallAtNode(const DataFacade<Algorithm> &facade,
3535
const NodeID to = facade.GetTarget(edge);
3636
const EdgeWeight edge_weight = data.weight;
3737
BOOST_ASSERT_MSG(edge_weight > 0, "edge_weight invalid");
38-
const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
38+
const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
3939
if (toHeapNode)
4040
{
4141
if (toHeapNode->weight + edge_weight < heapNode.weight)
@@ -64,7 +64,7 @@ void relaxOutgoingEdges(const DataFacade<Algorithm> &facade,
6464
BOOST_ASSERT_MSG(edge_weight > 0, "edge_weight invalid");
6565
const EdgeWeight to_weight = heapNode.weight + edge_weight;
6666

67-
auto toHeapNode = heap.GetHeapNodeIfWasInserted(to);
67+
const auto toHeapNode = heap.GetHeapNodeIfWasInserted(to);
6868
// New Node discovered -> Add to Heap + Node Info Storage
6969
if (!toHeapNode)
7070
{
@@ -124,7 +124,7 @@ void routingStep(const DataFacade<Algorithm> &facade,
124124
const bool force_loop_reverse)
125125
{
126126
auto heapNode = forward_heap.DeleteMinGetHeapNode();
127-
auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
127+
const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
128128

129129
if (reverseHeapNode)
130130
{

‎include/engine/routing_algorithms/routing_base_mld.hpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ void relaxOutgoingEdges(const DataFacade<Algorithm> &facade,
254254
{
255255
const EdgeWeight to_weight = heapNode.weight + shortcut_weight;
256256
BOOST_ASSERT(to_weight >= heapNode.weight);
257-
auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
257+
const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
258258
if (!toHeapNode)
259259
{
260260
forward_heap.Insert(to, to_weight, {heapNode.node, true});
@@ -283,7 +283,7 @@ void relaxOutgoingEdges(const DataFacade<Algorithm> &facade,
283283
{
284284
const EdgeWeight to_weight = heapNode.weight + shortcut_weight;
285285
BOOST_ASSERT(to_weight >= heapNode.weight);
286-
auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
286+
const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
287287
if (!toHeapNode)
288288
{
289289
forward_heap.Insert(to, to_weight, {heapNode.node, true});
@@ -321,7 +321,7 @@ void relaxOutgoingEdges(const DataFacade<Algorithm> &facade,
321321

322322
const EdgeWeight to_weight = heapNode.weight + node_weight + turn_penalty;
323323

324-
auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
324+
const auto toHeapNode = forward_heap.GetHeapNodeIfWasInserted(to);
325325
if (!toHeapNode)
326326
{
327327
forward_heap.Insert(to, to_weight, {heapNode.node, false});
@@ -357,7 +357,7 @@ void routingStep(const DataFacade<Algorithm> &facade,
357357
// is weight + reverse_weight
358358
// More tighter upper bound requires additional condition reverse_heap.WasRemoved(to)
359359
// with weight(to -> target) = reverse_weight and all weights ≥ 0
360-
auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
360+
const auto reverseHeapNode = reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
361361
if (reverseHeapNode)
362362
{
363363
auto reverse_weight = reverseHeapNode->weight;

‎src/contractor/contractor_search.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ void relaxNode(ContractorHeap &heap,
3232
}
3333
const EdgeWeight to_weight = node_weight + data.weight;
3434

35-
const auto& toHeapNode= heap.GetHeapNodeIfWasInserted(to);
35+
const auto toHeapNode= heap.GetHeapNodeIfWasInserted(to);
3636
// New Node discovered -> Add to Heap + Node Info Storage
3737
if (!toHeapNode)
3838
{

‎src/engine/routing_algorithms/alternative_path_ch.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ void alternativeRoutingStep(const DataFacade<Algorithm> &facade,
6262
QueryHeap &forward_heap = DIRECTION == FORWARD_DIRECTION ? heap1 : heap2;
6363
QueryHeap &reverse_heap = DIRECTION == FORWARD_DIRECTION ? heap2 : heap1;
6464

65-
auto & heapNode=forward_heap.DeleteMinGetHeapNode();
66-
const EdgeWeight weight = heapNode.weight;
65+
// Take a copy (no ref &) of the extracted node because otherwise could be modified later if toHeapNode is the same
66+
const auto heapNode=forward_heap.DeleteMinGetHeapNode();
6767

6868
const auto scaled_weight =
69-
static_cast<EdgeWeight>((weight + min_edge_offset) / (1. + VIAPATH_EPSILON));
69+
static_cast<EdgeWeight>((heapNode.weight + min_edge_offset) / (1. + VIAPATH_EPSILON));
7070
if ((INVALID_EDGE_WEIGHT != *upper_bound_to_shortest_path_weight) &&
7171
(scaled_weight > *upper_bound_to_shortest_path_weight))
7272
{
@@ -76,11 +76,11 @@ void alternativeRoutingStep(const DataFacade<Algorithm> &facade,
7676

7777
search_space.emplace_back(heapNode.data.parent, heapNode.node);
7878

79-
const auto& reverseHeapNode= reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
79+
const auto reverseHeapNode= reverse_heap.GetHeapNodeIfWasInserted(heapNode.node);
8080
if (reverseHeapNode)
8181
{
8282
search_space_intersection.emplace_back(heapNode.node);
83-
const EdgeWeight new_weight = reverseHeapNode->weight + weight;
83+
const EdgeWeight new_weight = reverseHeapNode->weight + heapNode.weight;
8484
if (new_weight < *upper_bound_to_shortest_path_weight)
8585
{
8686
if (new_weight >= 0)
@@ -112,9 +112,9 @@ void alternativeRoutingStep(const DataFacade<Algorithm> &facade,
112112
const EdgeWeight edge_weight = data.weight;
113113

114114
BOOST_ASSERT(edge_weight > 0);
115-
const EdgeWeight to_weight = weight + edge_weight;
115+
const EdgeWeight to_weight = heapNode.weight + edge_weight;
116116

117-
const auto& toHeapNode= forward_heap.GetHeapNodeIfWasInserted(to);
117+
const auto toHeapNode= forward_heap.GetHeapNodeIfWasInserted(to);
118118
// New Node discovered -> Add to Heap + Node Info Storage
119119
if (!toHeapNode)
120120
{

‎src/engine/routing_algorithms/many_to_many_ch.cpp

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ void relaxOutgoingEdges(const DataFacade<Algorithm> &facade,
7171
const auto to_duration = heapNode.data.duration + edge_duration;
7272
const auto to_distance = heapNode.data.distance + edge_distance;
7373

74-
const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
74+
const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
7575
// New Node discovered -> Add to Heap + Node Info Storage
7676
if (!toHeapNode)
7777
{
@@ -100,10 +100,8 @@ void forwardRoutingStep(const DataFacade<Algorithm> &facade,
100100
std::vector<NodeID> &middle_nodes_table,
101101
const PhantomNode &phantom_node)
102102
{
103-
const auto& heapNode=query_heap.DeleteMinGetHeapNode();
104-
const auto source_weight = heapNode.weight;
105-
const auto source_duration = heapNode.data.duration;
106-
const auto source_distance = heapNode.data.distance;
103+
// Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same
104+
const auto heapNode=query_heap.DeleteMinGetHeapNode();
107105

108106
// Check if each encountered node has an entry
109107
const auto &bucket_list = std::equal_range(search_space_with_buckets.begin(),
@@ -128,9 +126,9 @@ void forwardRoutingStep(const DataFacade<Algorithm> &facade,
128126
: distances_table[row_index * number_of_targets + column_index];
129127

130128
// Check if new weight is better
131-
auto new_weight = source_weight + target_weight;
132-
auto new_duration = source_duration + target_duration;
133-
auto new_distance = source_distance + target_distance;
129+
auto new_weight = heapNode.weight + target_weight;
130+
auto new_duration = heapNode.data.duration + target_duration;
131+
auto new_distance = heapNode.data.distance + target_distance;
134132

135133
if (new_weight < 0)
136134
{
@@ -161,15 +159,12 @@ void backwardRoutingStep(const DataFacade<Algorithm> &facade,
161159
std::vector<NodeBucket> &search_space_with_buckets,
162160
const PhantomNode &phantom_node)
163161
{
162+
// Take a copy (no ref &) of the extracted node because otherwise could be modified later if toHeapNode is the same
164163
const auto heapNode=query_heap.DeleteMinGetHeapNode();
165-
const auto target_weight = heapNode.weight;
166-
const auto target_duration = heapNode.data.duration;
167-
const auto target_distance = heapNode.data.distance;
168-
const auto parent = heapNode.data.parent;
169164

170165
// Store settled nodes in search space bucket
171166
search_space_with_buckets.emplace_back(
172-
heapNode.node, parent, column_index, target_weight, target_duration, target_distance);
167+
heapNode.node, heapNode.data.parent, column_index, heapNode.weight, heapNode.data.duration, heapNode.data.distance);
173168

174169
relaxOutgoingEdges<REVERSE_DIRECTION>(
175170
facade, heapNode, query_heap, phantom_node);

‎src/engine/routing_algorithms/many_to_many_mld.cpp

Lines changed: 34 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ void relaxBorderEdges(const DataFacade<mld::Algorithm> &facade,
7171
const auto to_distance = distance + node_distance;
7272

7373
// New Node discovered -> Add to Heap + Node Info Storage
74-
const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
74+
const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
7575
if (!toHeapNode)
7676
{
7777
query_heap.Insert(to, to_weight, {node, false, to_duration, to_distance});
@@ -93,11 +93,11 @@ void relaxBorderEdges(const DataFacade<mld::Algorithm> &facade,
9393

9494
template <bool DIRECTION, typename... Args>
9595
void relaxOutgoingEdges(const DataFacade<mld::Algorithm> &facade,
96-
const SearchEngineData<mld::Algorithm>::ManyToManyQueryHeap::HeapNode& heapNode,
96+
const typename SearchEngineData<mld::Algorithm>::ManyToManyQueryHeap::HeapNode& heapNode,
9797
typename SearchEngineData<mld::Algorithm>::ManyToManyQueryHeap &query_heap,
9898
Args... args)
9999
{
100-
BOOST_ASSERT(!facade.ExcludeNode(heapNode.node));
100+
BOOST_ASSERT(!facade.ExcludeNode(node));
101101

102102
const auto &partition = facade.GetMultiLevelPartition();
103103

@@ -109,40 +109,40 @@ void relaxOutgoingEdges(const DataFacade<mld::Algorithm> &facade,
109109

110110
const auto &cells = facade.GetCellStorage();
111111
const auto &metric = facade.GetCellMetric();
112-
const auto node=heapNode.node;
112+
113113

114114
if (level >= 1 && !heapNode.data.from_clique_arc)
115115
{
116-
const auto &cell = cells.GetCell(metric, level, partition.GetCell(level, node));
116+
const auto &cell = cells.GetCell(metric, level, partition.GetCell(level, heapNode.node));
117117
if (DIRECTION == FORWARD_DIRECTION)
118118
{ // Shortcuts in forward direction
119119
auto destination = cell.GetDestinationNodes().begin();
120-
auto shortcut_durations = cell.GetOutDuration(node);
121-
auto shortcut_distances = cell.GetOutDistance(node);
122-
for (auto shortcut_weight : cell.GetOutWeight(node))
120+
auto shortcut_durations = cell.GetOutDuration(heapNode.node);
121+
auto shortcut_distances = cell.GetOutDistance(heapNode.node);
122+
for (auto shortcut_weight : cell.GetOutWeight(heapNode.node))
123123
{
124124
BOOST_ASSERT(destination != cell.GetDestinationNodes().end());
125125
BOOST_ASSERT(!shortcut_durations.empty());
126126
BOOST_ASSERT(!shortcut_distances.empty());
127127
const NodeID to = *destination;
128128

129-
if (shortcut_weight != INVALID_EDGE_WEIGHT && node != to)
129+
if (shortcut_weight != INVALID_EDGE_WEIGHT && heapNode.node != to)
130130
{
131131
const auto to_weight = heapNode.weight + shortcut_weight;
132132
const auto to_duration = heapNode.data.duration + shortcut_durations.front();
133133
const auto to_distance = heapNode.data.distance + shortcut_distances.front();
134-
const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
134+
const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
135135
if (!toHeapNode)
136136
{
137-
query_heap.Insert(to, to_weight, {node, true, to_duration, to_distance});
137+
query_heap.Insert(to, to_weight, {heapNode.node, true, to_duration, to_distance});
138138
}
139-
else if (std::tie(to_weight, to_duration, to_distance, node) <
139+
else if (std::tie(to_weight, to_duration, to_distance, heapNode.node) <
140140
std::tie(toHeapNode->weight,
141141
toHeapNode->data.duration,
142142
toHeapNode->data.distance,
143143
toHeapNode->data.parent))
144144
{
145-
toHeapNode->data = {node, true, to_duration, to_distance};
145+
toHeapNode->data = {heapNode.node, true, to_duration, to_distance};
146146
toHeapNode->weight=to_weight;
147147
query_heap.DecreaseKey(*toHeapNode);
148148
}
@@ -157,32 +157,32 @@ void relaxOutgoingEdges(const DataFacade<mld::Algorithm> &facade,
157157
else
158158
{ // Shortcuts in backward direction
159159
auto source = cell.GetSourceNodes().begin();
160-
auto shortcut_durations = cell.GetInDuration(node);
161-
auto shortcut_distances = cell.GetInDistance(node);
162-
for (auto shortcut_weight : cell.GetInWeight(node))
160+
auto shortcut_durations = cell.GetInDuration(heapNode.node);
161+
auto shortcut_distances = cell.GetInDistance(heapNode.node);
162+
for (auto shortcut_weight : cell.GetInWeight(heapNode.node))
163163
{
164164
BOOST_ASSERT(source != cell.GetSourceNodes().end());
165165
BOOST_ASSERT(!shortcut_durations.empty());
166166
BOOST_ASSERT(!shortcut_distances.empty());
167167
const NodeID to = *source;
168168

169-
if (shortcut_weight != INVALID_EDGE_WEIGHT && node != to)
169+
if (shortcut_weight != INVALID_EDGE_WEIGHT && heapNode.node != to)
170170
{
171171
const auto to_weight = heapNode.weight + shortcut_weight;
172172
const auto to_duration = heapNode.data.duration + shortcut_durations.front();
173173
const auto to_distance = heapNode.data.distance + shortcut_distances.front();
174-
const auto& toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
174+
const auto toHeapNode= query_heap.GetHeapNodeIfWasInserted(to);
175175
if (!toHeapNode)
176176
{
177-
query_heap.Insert(to, to_weight, {node, true, to_duration, to_distance});
177+
query_heap.Insert(to, to_weight, {heapNode.node, true, to_duration, to_distance});
178178
}
179-
else if (std::tie(to_weight, to_duration, to_distance, node) <
179+
else if (std::tie(to_weight, to_duration, to_distance, heapNode.node) <
180180
std::tie(toHeapNode->weight,
181181
toHeapNode->data.duration,
182182
toHeapNode->data.distance,
183183
toHeapNode->data.parent))
184184
{
185-
toHeapNode->data = {node, true, to_duration, to_distance};
185+
toHeapNode->data = {heapNode.node, true, to_duration, to_distance};
186186
toHeapNode->weight=to_weight;
187187
query_heap.DecreaseKey(*toHeapNode);
188188
}
@@ -196,7 +196,7 @@ void relaxOutgoingEdges(const DataFacade<mld::Algorithm> &facade,
196196
}
197197
}
198198

199-
relaxBorderEdges<DIRECTION>(facade, node, heapNode.weight, heapNode.data.duration, heapNode.data.distance, query_heap, level);
199+
relaxBorderEdges<DIRECTION>(facade, heapNode.node, heapNode.weight, heapNode.data.duration, heapNode.data.distance, query_heap, level);
200200
}
201201

202202
//
@@ -371,14 +371,12 @@ oneToManySearch(SearchEngineData<Algorithm> &engine_working_data,
371371

372372
while (!query_heap.Empty() && !target_nodes_index.empty())
373373
{
374-
// Extract node from the heap
375-
const auto& heapNode=query_heap.DeleteMinGetHeapNode();
376-
const auto weight = heapNode.weight;
377-
const auto duration = heapNode.data.duration;
378-
const auto distance = heapNode.data.distance;
374+
// Extract node from the heap. Take a copy (no ref) because otherwise can be modified later
375+
// if toHeapNode is the same
376+
const auto heapNode=query_heap.DeleteMinGetHeapNode();
379377

380378
// Update values
381-
update_values(heapNode.node, weight, duration, distance);
379+
update_values(heapNode.node, heapNode.weight, heapNode.data.duration, heapNode.data.distance);
382380

383381
// Relax outgoing edges
384382
relaxOutgoingEdges<DIRECTION>(facade,
@@ -408,10 +406,8 @@ void forwardRoutingStep(const DataFacade<Algorithm> &facade,
408406
std::vector<NodeID> &middle_nodes_table,
409407
const PhantomNode &phantom_node)
410408
{
411-
const auto& heapNode=query_heap.DeleteMinGetHeapNode();
412-
const auto source_weight = heapNode.weight;
413-
const auto source_duration = heapNode.data.duration;
414-
const auto source_distance = heapNode.data.distance;
409+
// Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same
410+
const auto heapNode=query_heap.DeleteMinGetHeapNode();
415411

416412
// Check if each encountered node has an entry
417413
const auto &bucket_list = std::equal_range(search_space_with_buckets.begin(),
@@ -439,9 +435,9 @@ void forwardRoutingStep(const DataFacade<Algorithm> &facade,
439435
auto &current_distance = distances_table.empty() ? nulldistance : distances_table[location];
440436

441437
// Check if new weight is better
442-
auto new_weight = source_weight + target_weight;
443-
auto new_duration = source_duration + target_duration;
444-
auto new_distance = source_distance + target_distance;
438+
auto new_weight = heapNode.weight + target_weight;
439+
auto new_duration = heapNode.data.duration + target_duration;
440+
auto new_distance = heapNode.data.distance + target_distance;
445441

446442
if (new_weight >= 0 &&
447443
std::tie(new_weight, new_duration, new_distance) <
@@ -465,16 +461,12 @@ void backwardRoutingStep(const DataFacade<Algorithm> &facade,
465461
std::vector<NodeBucket> &search_space_with_buckets,
466462
const PhantomNode &phantom_node)
467463
{
468-
const auto& heapNode=query_heap.DeleteMinGetHeapNode();
469-
const auto target_weight = heapNode.weight;
470-
const auto target_duration = heapNode.data.duration;
471-
const auto target_distance = heapNode.data.distance;
472-
const auto parent = heapNode.data.parent;
473-
const auto from_clique_arc = heapNode.data.from_clique_arc;
464+
// Take a copy of the extracted node because otherwise could be modified later if toHeapNode is the same
465+
const auto heapNode=query_heap.DeleteMinGetHeapNode();
474466

475467
// Store settled nodes in search space bucket
476468
search_space_with_buckets.emplace_back(
477-
heapNode.node, parent, from_clique_arc, column_idx, target_weight, target_duration, target_distance);
469+
heapNode.node, heapNode.data.parent, heapNode.data.from_clique_arc, column_idx, heapNode.weight, heapNode.data.duration, heapNode.data.distance);
478470

479471
const auto &partition = facade.GetMultiLevelPartition();
480472
const auto maximal_level = partition.GetNumberOfLevels() - 1;

0 commit comments

Comments
 (0)