terraform/rootmodule: Make walker async by default

internal/context/context.go

@@ -30,6 +30,7 @@ var (
 	ctxParserFinder     = &contextKey{"parser finder"}
 	ctxTfExecFinder     = &contextKey{"terraform exec finder"}
 	ctxRootModuleCaFi   = &contextKey{"root module candidate finder"}
+	ctxRootModuleWalker = &contextKey{"root module walker"}
 	ctxRootDir          = &contextKey{"root directory"}
 )
 
@@ -185,3 +186,15 @@ func RootDirectory(ctx context.Context) (string, bool) {
 	}
 	return *rootDir, true
 }
+
+func WithRootModuleWalker(w *rootmodule.Walker, ctx context.Context) context.Context {
+	return context.WithValue(ctx, ctxRootModuleWalker, w)
+}
+
+func RootModuleWalker(ctx context.Context) (*rootmodule.Walker, error) {
+	w, ok := ctx.Value(ctxRootModuleWalker).(*rootmodule.Walker)
+	if !ok {
+		return nil, missingContextErr(ctxRootModuleWalker)
+	}
+	return w, nil
+}

internal/terraform/rootmodule/root_module.go

@@ -59,11 +59,7 @@ func NewRootModule(ctx context.Context, dir string) (RootModule, error) {
 	rm.tfDiscoFunc = d.LookPath
 
 	rm.tfNewExecutor = exec.NewExecutor
-	rm.newSchemaStorage = func() *schema.Storage {
-		ss := schema.NewStorage()
-		ss.SetSynchronous()
-		return ss
-	}
+	rm.newSchemaStorage = schema.NewStorage
 
 	return rm, rm.init(ctx)
 }
@@ -281,8 +277,15 @@ func (rm *rootModule) UpdatePluginCache(lockFile File) error {
 
 	rm.pluginLockFile = lockFile
 
-	return rm.schemaWriter.ObtainSchemasForModule(
+	err := rm.schemaWriter.ObtainSchemasForModule(
 		rm.tfExec, rootModuleDirFromFilePath(lockFile.Path()))
+	if err != nil {
+		// We fail silently here to still allow tracking the module
+		// The schema can be loaded later via watcher
+		rm.logger.Printf("failed to update plugin cache: %s", err.Error())
+	}
+
+	return nil
 }
 
 func (rm *rootModule) PathsToWatch() []string {

internal/terraform/rootmodule/root_module_manager.go

@@ -71,23 +71,23 @@ func (rmm *rootModuleManager) SetLogger(logger *log.Logger) {
 	rmm.logger = logger
 }
 
-func (rmm *rootModuleManager) AddRootModule(dir string) error {
+func (rmm *rootModuleManager) AddRootModule(dir string) (RootModule, error) {
 	dir = filepath.Clean(dir)
 
 	// TODO: Follow symlinks (requires proper test data)
 
 	if rmm.exists(dir) {
-		return fmt.Errorf("root module %s was already added", dir)
+		return nil, fmt.Errorf("root module %s was already added", dir)
 	}
 
 	rm, err := rmm.newRootModule(context.Background(), dir)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	rmm.rms = append(rmm.rms, rm)
 
-	return nil
+	return rm, nil
 }
 
 func (rmm *rootModuleManager) exists(dir string) bool {

internal/terraform/rootmodule/root_module_manager_mock.go

@@ -44,11 +44,7 @@ func NewRootModuleMock(ctx context.Context, rmm *RootModuleMock, dir string) *ro
 	rm.tfNewExecutor = exec.MockExecutor(rmm.TerraformExecQueue)
 
 	if rmm.ProviderSchemas == nil {
-		rm.newSchemaStorage = func() *schema.Storage {
-			ss := schema.NewStorage()
-			ss.SetSynchronous()
-			return ss
-		}
+		rm.newSchemaStorage = schema.NewStorage
 	} else {
 		rm.newSchemaStorage = schema.MockStorage(rmm.ProviderSchemas)
 	}

internal/terraform/rootmodule/root_module_manager_mock_test.go

@@ -12,7 +12,7 @@ import (
 func TestNewRootModuleManagerMock_noMocks(t *testing.T) {
 	f := NewRootModuleManagerMock(map[string]*RootModuleMock{})
 	rmm := f(context.Background())
-	err := rmm.AddRootModule("any-path")
+	_, err := rmm.AddRootModule("any-path")
 	if err == nil {
 		t.Fatal("expected unmocked path addition to fail")
 	}
@@ -38,7 +38,7 @@ func TestNewRootModuleManagerMock_mocks(t *testing.T) {
 		},
 	})
 	rmm := f(context.Background())
-	err := rmm.AddRootModule(tmpDir)
+	_, err := rmm.AddRootModule(tmpDir)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/terraform/rootmodule/root_module_manager_test.go

@@ -474,9 +474,10 @@ func TestRootModuleManager_RootModuleCandidatesByPath(t *testing.T) {
 		base := filepath.Base(tc.walkerRoot)
 		t.Run(fmt.Sprintf("%s/%d-%s", base, i, tc.name), func(t *testing.T) {
 			rmm := testRootModuleManager(t)
-			w := NewWalker()
+			w := MockWalker()
 			err := w.WalkInitializedRootModules(tc.walkerRoot, func(rmPath string) error {
-				return rmm.AddRootModule(rmPath)
+				_, err := rmm.AddRootModule(rmPath)
+				return err
 			})
 			if err != nil {
 				t.Fatal(err)

internal/terraform/rootmodule/types.go

@@ -34,7 +34,7 @@ type RootModuleManager interface {
 	SetTerraformExecPath(path string)
 	SetTerraformExecLogPath(logPath string)
 	SetTerraformExecTimeout(timeout time.Duration)
-	AddRootModule(dir string) error
+	AddRootModule(dir string) (RootModule, error)
 	PathsToWatch() []string
 	RootModuleByPath(path string) (RootModule, error)
 }
@@ -52,3 +52,5 @@ type RootModule interface {
 type RootModuleFactory func(context.Context, string) (*rootModule, error)
 
 type RootModuleManagerFactory func(context.Context) RootModuleManager
+
+type WalkerFactory func() *Walker

internal/terraform/rootmodule/walker.go

@@ -1,6 +1,7 @@
 package rootmodule
 
 import (
+	"fmt"
 	"io/ioutil"
 	"log"
 	"os"
@@ -21,12 +22,16 @@ var (
 )
 
 type Walker struct {
-	logger *log.Logger
+	logger  *log.Logger
+	sync    bool
+	walking bool
+	doneCh  chan struct{}
 }
 
 func NewWalker() *Walker {
 	return &Walker{
 		logger: discardLogger,
+		doneCh: make(chan struct{}, 0),
 	}
 }
 
@@ -36,9 +41,45 @@ func (w *Walker) SetLogger(logger *log.Logger) {
 
 type WalkFunc func(rootModulePath string) error
 
+func (w *Walker) Stop() {
+	if w.walking {
+		w.walking = false
+		w.doneCh <- struct{}{}
+	}
+}
+
 func (w *Walker) WalkInitializedRootModules(path string, wf WalkFunc) error {
-	w.logger.Printf("walking through %s", path)
-	return filepath.Walk(path, func(path string, info os.FileInfo, err error) error {
+	if w.walking {
+		return fmt.Errorf("walker is already running")
+	}
+	w.walking = true
+	if w.sync {
+		w.logger.Printf("synchronously walking through %s", path)
+		return w.walk(path, wf)
+	}
+
+	go func(w *Walker, path string, wf WalkFunc) {
+		w.logger.Printf("asynchronously walking through %s", path)
+		err := w.walk(path, wf)
+		if err != nil {
+			w.logger.Printf("async walking through %s failed: %s", path, err)
+			return
+		}
+		w.logger.Printf("async walking through %s finished", path)
+	}(w, path, wf)
+
+	return nil
+}
+
+func (w *Walker) walk(rootPath string, wf WalkFunc) error {
+	err := filepath.Walk(rootPath, func(path string, info os.FileInfo, err error) error {
+		select {
+		case <-w.doneCh:
+			w.logger.Printf("cancelling walk of %s...", rootPath)
+			return fmt.Errorf("walk cancelled")
+		default:
+		}
+
 		if err != nil {
 			w.logger.Printf("unable to access %s: %s", path, err.Error())
 			return nil
@@ -66,6 +107,8 @@ func (w *Walker) WalkInitializedRootModules(path string, wf WalkFunc) error {
 
 		return nil
 	})
+	w.walking = false
+	return err
 }
 
 func isSkippableDir(dirName string) bool {

internal/terraform/rootmodule/walker_mock.go

@@ -0,0 +1,7 @@
+package rootmodule
+
+func MockWalker() *Walker {
+	w := NewWalker()
+	w.sync = true
+	return w
+}

internal/terraform/schema/schema_storage.go

@@ -51,9 +51,6 @@ type Storage struct {
 	// sem ensures atomic reading and obtaining of schemas
 	// as the process of obtaining it may not be thread-safe
 	sem *semaphore.Weighted
-
-	// sync makes operations synchronous which makes testing easier
-	sync bool
 }
 
 var defaultLogger = log.New(ioutil.Discard, "", 0)
@@ -102,28 +99,10 @@ func (s *Storage) SetLogger(logger *log.Logger) {
 	s.logger = logger
 }
 
-func (s *Storage) SetSynchronous() {
-	s.sync = true
-}
-
 // ObtainSchemasForModule will (by default) asynchronously obtain schema via tf
 // and store it for later consumption via Reader methods
 func (s *Storage) ObtainSchemasForModule(tf *exec.Executor, dir string) error {
-	if s.sync {
-		return s.obtainSchemasForModule(tf, dir)
-	}
-
-	// This routine is not cancellable in itself
-	// but the time-consuming part is done by exec.Executor
-	// which is cancellable via its own context
-	go func() {
-		err := s.obtainSchemasForModule(tf, dir)
-		if err != nil {
-			s.logger.Printf("error obtaining schemas for %s: %s", dir, err)
-		}
-	}()
-
-	return nil
+	return s.obtainSchemasForModule(tf, dir)
 }
 
 func (s *Storage) obtainSchemasForModule(tf *exec.Executor, dir string) error {

internal/terraform/schema/schema_storage_mock.go

@@ -11,7 +11,6 @@ func MockStorage(ps *tfjson.ProviderSchemas) StorageFactory {
 			ps = &tfjson.ProviderSchemas{}
 		}
 		s.ps = ps
-		s.sync = true
 		return s
 	}
 }

langserver/handlers/handlers_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/terraform-ls/internal/lsp"
 	"github.com/hashicorp/terraform-ls/internal/terraform/exec"
@@ -79,8 +80,12 @@ func TestEOF(t *testing.T) {
 
 	ls.CloseClientStdout(t)
 
+	// Session is stopped after all other operations stop
+	// which may take some time
+	time.Sleep(1 * time.Millisecond)
+
 	if !ms.StopFuncCalled() {
-		t.Fatal("Expected service to stop on EOF")
+		t.Fatal("Expected session to stop on EOF")
 	}
 	if ls.StopFuncCalled() {
 		t.Fatal("Expected server not to stop on EOF")

langserver/handlers/initialize.go

@@ -6,7 +6,6 @@ import (
 
 	lsctx "github.com/hashicorp/terraform-ls/internal/context"
 	ilsp "github.com/hashicorp/terraform-ls/internal/lsp"
-	"github.com/hashicorp/terraform-ls/internal/terraform/rootmodule"
 	lsp "github.com/sourcegraph/go-lsp"
 )
 
@@ -55,25 +54,28 @@ func (lh *logHandler) Initialize(ctx context.Context, params lsp.InitializeParam
 		return serverCaps, err
 	}
 
-	walker := rootmodule.NewWalker()
-	walker.SetLogger(lh.logger)
-	err = walker.WalkInitializedRootModules(fh.Dir(), func(dir string) error {
-		lh.logger.Printf("Adding root module (via %T): %s", rmm, dir)
-		return rmm.AddRootModule(dir)
-	})
+	walker, err := lsctx.RootModuleWalker(ctx)
 	if err != nil {
 		return serverCaps, err
 	}
 
-	err = w.AddPaths(rmm.PathsToWatch())
-	if err != nil {
-		return serverCaps, err
-	}
+	walker.SetLogger(lh.logger)
+	err = walker.WalkInitializedRootModules(fh.Dir(), func(dir string) error {
+		lh.logger.Printf("Adding root module: %s", dir)
+		rm, err := rmm.AddRootModule(dir)
+		if err != nil {
+			return err
+		}
 
-	err = w.Start()
-	if err != nil {
-		return serverCaps, err
-	}
+		paths := rm.PathsToWatch()
+		lh.logger.Printf("Adding %d paths of root module for watching (%s)", len(paths), dir)
+		err = w.AddPaths(paths)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
 
-	return serverCaps, nil
+	return serverCaps, err
 }