]> granicus.if.org Git - graphviz/commitdiff
get_cycle_centroid: undo caching of 'cycle' list
authorMatthew Fernandez <matthew.fernandez@gmail.com>
Mon, 17 Jan 2022 16:09:11 +0000 (08:09 -0800)
committerMatthew Fernandez <matthew.fernandez@gmail.com>
Tue, 18 Jan 2022 15:42:37 +0000 (07:42 -0800)
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.

lib/common/routespl.c

index 9d31d598b9c454097e49d78270e24dd67cd4d0fb..d8715af245f953f8c26cc5068ff53ebe9bdf044a 100644 (file)
@@ -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;