From: Matthew Fernandez <matthew.fernandez@gmail.com>
Date: Mon, 17 Jan 2022 16:09:11 +0000 (-0800)
Subject: get_cycle_centroid: undo caching of 'cycle' list
X-Git-Tag: 3.0.0~60^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e982a7f8dc7367911b823c81ca527629575f5ec4;p=graphviz

get_cycle_centroid: undo caching of 'cycle' list

This function attempted to save time by caching the results of a graph cycle
search and then reusing them in a future call. The logic used for this caching
is invalid. It compares graph pointers to see if it is operating on the same
graph as last time, but this can return a false positive if the previous graph
has been deallocated and a new (unrelated) graph has been allocated at the same
address.

This logic would eventually have to be unwound or adjusted in order to support
multithreading in this code too.

It is possible this is the root cause of the issue described in #2162, though I
was never able to reproduce that issue.
---

diff --git a/lib/common/routespl.c b/lib/common/routespl.c
index 9d31d598b..d8715af24 100644
--- a/lib/common/routespl.c
+++ b/lib/common/routespl.c
@@ -1008,17 +1008,7 @@ static vec* find_shortest_cycle_with_edge(vec* cycles, edge_t* edge, size_t min_
 
 static pointf get_cycle_centroid(graph_t *g, edge_t* edge)
 {
-	static vec* cycles = 0;
-	static graph_t* ref_g = 0;
-
-	if (cycles == 0 || ref_g != g) {
-		//free the memory we're using to hold the previous cycles
-		if (cycles != 0) {
-			vec_delete(cycles);
-		}
-		cycles = find_all_cycles(g);
-		ref_g = g;
-	}
+	vec* cycles = find_all_cycles(g);
 
 	//find the center of the shortest cycle containing this edge
 	//cycles of length 2 do their own thing, we want 3 or
@@ -1029,8 +1019,11 @@ static pointf get_cycle_centroid(graph_t *g, edge_t* edge)
 	size_t idx; //edge index
 	node_t *n;
 
-	if (!cycle)
+	if (cycle == NULL) {
+		if (cycles != NULL)
+			vec_delete(cycles);
 		return get_centroid(g);
+	}
 
 	cycle_len = vec_length(cycle);
 
@@ -1041,6 +1034,9 @@ static pointf get_cycle_centroid(graph_t *g, edge_t* edge)
         cnt++;
 	}
 
+	if (cycles != NULL)
+		vec_delete(cycles);
+
 	sum.x /= cnt;
     sum.y /= cnt;
     return sum;