From a5c8b249f8bd21a9e2a8501a6a12b94ef2a4dd5a Mon Sep 17 00:00:00 2001 From: Ahsan Barkati Date: Wed, 28 Oct 2020 10:19:45 +0530 Subject: [PATCH] fix(Query): Fix wrong path response for k-shortest paths (#6437) (#6791) The k-shortest path query sometimes returns a wrong response. The cause of this issue is inappropriate use of sync.pools. Whenever a valid path is found a new variable is assigned with this route and that variable is appended in the kroutes (which is then parsed to produce response). But the route is also put in the sync.pool. Since the route is a structure containing a pointer to slice along with other fields, when it is fetched back from the pool in future then any mutation on this fetched route causes modification in the kroutes, resulting in garbage response. This issue is fixed by making a copy of the route struct rather than just assigning it to a variable, which causes a shallow copy. (cherry picked from commit 4792d8b1c4391a08ce2627f9615ed12699f2885f) --- query/shortest.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/query/shortest.go b/query/shortest.go index 35505af5e35..2ece3c232fc 100644 --- a/query/shortest.go +++ b/query/shortest.go @@ -340,8 +340,13 @@ func runKShortestPaths(ctx context.Context, sg *SubGraph) ([]*SubGraph, error) { continue } - // Add path to list. + // Add path to list after making a copy of the path in itemRoute. A copy of + // *item.path.route is required because it has to be put back in the sync pool and a + // future reuse can alter the item already present in kroute because it is a pointer. + itemRoute := make([]pathInfo, len(*item.path.route)) + copy(itemRoute, *item.path.route) newRoute := item.path + newRoute.route = &itemRoute newRoute.totalWeight = item.cost kroutes = append(kroutes, newRoute) if len(kroutes) == numPaths {