From a23b945db71afba631daba405a3062a3bda81102 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Tue, 29 Aug 2023 08:10:47 -0400 Subject: [PATCH] Fix world's slowest stack overflow (#123) * Fix world's slowest stack overflow When dealing with CRDsonnet libraries in our (Grafana Labs) repositories, the language server would lock up: - I thought it had something to do with CRDsonnet but I was wrong. - This happened because it was trying to find a field in the wrong nodestack repeatedly (see image) - This PR fixes the issue. - Obviously, the language server can't find fields that come from processing jsonschema, but at least it doesn't take up 4 full CPUs (it reached a full 1% CPU usage while testing :) ) * Remove unneeded boolean All tests are still passing. `$` can be resolved across multiple files, so I guess the language server is just better now and doesn't need that safety If this causes a bug, we should add a new test --- pkg/ast/processing/find_field.go | 20 +++++++++++-------- pkg/server/definition_test.go | 16 +++++++++++++++ .../goto-infinite-recursion-bug-1.jsonnet | 6 ++++++ .../goto-infinite-recursion-bug-2.libsonnet | 15 ++++++++++++++ .../goto-infinite-recursion-bug-3.libsonnet | 13 ++++++++++++ 5 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 pkg/server/testdata/goto-infinite-recursion-bug-1.jsonnet create mode 100644 pkg/server/testdata/goto-infinite-recursion-bug-2.libsonnet create mode 100644 pkg/server/testdata/goto-infinite-recursion-bug-3.libsonnet diff --git a/pkg/ast/processing/find_field.go b/pkg/ast/processing/find_field.go index c674044..364be83 100644 --- a/pkg/ast/processing/find_field.go +++ b/pkg/ast/processing/find_field.go @@ -15,7 +15,6 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm var foundDesugaredObjects []*ast.DesugaredObject // First element will be super, self, or var name start, indexList := indexList[0], indexList[1:] - sameFileOnly := false switch { case start == "super": // Find the LHS desugared object of a binary node @@ -36,7 +35,6 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm case start == "std": return nil, fmt.Errorf("cannot get definition of std lib") case start == "$": - sameFileOnly = true foundDesugaredObjects = FindTopLevelObjects(nodestack.NewNodeStack(stack.From), vm) case strings.Contains(start, "."): foundDesugaredObjects = FindTopLevelObjectsInFile(vm, start, "") @@ -66,6 +64,7 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm case *ast.Import: filename := bodyNode.File.Value foundDesugaredObjects = FindTopLevelObjectsInFile(vm, filename, "") + case *ast.Index, *ast.Apply: tempStack := nodestack.NewNodeStack(bodyNode) indexList = append(tempStack.BuildIndexList(), indexList...) @@ -80,10 +79,10 @@ func FindRangesFromIndexList(stack *nodestack.NodeStack, indexList []string, vm } } - return extractObjectRangesFromDesugaredObjs(stack, vm, foundDesugaredObjects, sameFileOnly, indexList, partialMatchFields) + return extractObjectRangesFromDesugaredObjs(vm, foundDesugaredObjects, indexList, partialMatchFields) } -func extractObjectRangesFromDesugaredObjs(stack *nodestack.NodeStack, vm *jsonnet.VM, desugaredObjs []*ast.DesugaredObject, sameFileOnly bool, indexList []string, partialMatchFields bool) ([]ObjectRange, error) { +func extractObjectRangesFromDesugaredObjs(vm *jsonnet.VM, desugaredObjs []*ast.DesugaredObject, indexList []string, partialMatchFields bool) ([]ObjectRange, error) { var ranges []ObjectRange for len(indexList) > 0 { index := indexList[0] @@ -135,10 +134,15 @@ func extractObjectRangesFromDesugaredObjs(stack *nodestack.NodeStack, vm *jsonne case *ast.DesugaredObject: desugaredObjs = append(desugaredObjs, fieldNode) case *ast.Index: - additionalIndexList := append(nodestack.NewNodeStack(fieldNode).BuildIndexList(), indexList...) - result, err := FindRangesFromIndexList(stack, additionalIndexList, vm, partialMatchFields) - if len(result) > 0 { - if !sameFileOnly || result[0].Filename == stack.From.Loc().FileName { + // if we're trying to find the a definition which is an index, + // we need to find it from itself, meaning that we need to create a stack + // from the index's target and search from there + rootNode, _, _ := vm.ImportAST("", fieldNode.LocRange.FileName) + stack, _ := FindNodeByPosition(rootNode, fieldNode.Target.Loc().Begin) + if stack != nil { + additionalIndexList := append(nodestack.NewNodeStack(fieldNode).BuildIndexList(), indexList...) + result, _ := FindRangesFromIndexList(stack, additionalIndexList, vm, partialMatchFields) + if len(result) > 0 { return result, err } } diff --git a/pkg/server/definition_test.go b/pkg/server/definition_test.go index d2908ca..6661dba 100644 --- a/pkg/server/definition_test.go +++ b/pkg/server/definition_test.go @@ -910,6 +910,22 @@ var definitionTestCases = []definitionTestCase{ }, }}, }, + { + name: "test fix infinite recursion", + filename: "./testdata/goto-infinite-recursion-bug-1.jsonnet", + position: protocol.Position{Line: 2, Character: 26}, + results: []definitionResult{{ + targetFilename: "testdata/goto-infinite-recursion-bug-3.libsonnet", + targetRange: protocol.Range{ + Start: protocol.Position{Line: 5, Character: 10}, + End: protocol.Position{Line: 5, Character: 36}, + }, + targetSelectionRange: protocol.Range{ + Start: protocol.Position{Line: 5, Character: 10}, + End: protocol.Position{Line: 5, Character: 22}, + }, + }}, + }, } func TestDefinition(t *testing.T) { diff --git a/pkg/server/testdata/goto-infinite-recursion-bug-1.jsonnet b/pkg/server/testdata/goto-infinite-recursion-bug-1.jsonnet new file mode 100644 index 0000000..a1592c5 --- /dev/null +++ b/pkg/server/testdata/goto-infinite-recursion-bug-1.jsonnet @@ -0,0 +1,6 @@ +local drone = import './goto-infinite-recursion-bug-2.libsonnet'; +{ + steps: drone.step.withCommands([ + 'blabla', + ]), +} diff --git a/pkg/server/testdata/goto-infinite-recursion-bug-2.libsonnet b/pkg/server/testdata/goto-infinite-recursion-bug-2.libsonnet new file mode 100644 index 0000000..53b2dd4 --- /dev/null +++ b/pkg/server/testdata/goto-infinite-recursion-bug-2.libsonnet @@ -0,0 +1,15 @@ +local drone = import './goto-infinite-recursion-bug-3.libsonnet'; +{ + pipeline: + drone.pipeline.docker { + new(name): + super.new(name) + + super.clone.withRetries(3) + + super.clone.withDepth(10000) + + super.platform.withArch('amd64') + + super.withImagePullSecrets(['dockerconfigjson']), + }, + + step: + drone.pipeline.docker.step, +} diff --git a/pkg/server/testdata/goto-infinite-recursion-bug-3.libsonnet b/pkg/server/testdata/goto-infinite-recursion-bug-3.libsonnet new file mode 100644 index 0000000..ddc49ee --- /dev/null +++ b/pkg/server/testdata/goto-infinite-recursion-bug-3.libsonnet @@ -0,0 +1,13 @@ +local lib = + { + pipeline: { + docker: { + step: { + withCommands(commands): {}, + }, + }, + }, + }; + + +lib