From ecaf9461cef875221155a997c1cf6371a5482b49 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Fri, 28 Aug 2020 18:54:00 +0200 Subject: [PATCH] loader: be more robust when creating the cached GOROOT This commit fixes two issues: * Do not try to create the cached GOROOT multiple times in parallel. This may happen in tests and is a waste of resources (and thus speed). * Check for an "access denied" error when trying to rename a directory over an existing directory. On *nix systems, this results in the expected "file exists" error. Unfortunately, Windows gives an access denied. This commit fixes the Windows behavior. --- loader/goroot.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/loader/goroot.go b/loader/goroot.go index d1c5d1c3..c40a9f3a 100644 --- a/loader/goroot.go +++ b/loader/goroot.go @@ -15,11 +15,14 @@ import ( "path" "path/filepath" "runtime" + "sync" "github.com/tinygo-org/tinygo/compileopts" "github.com/tinygo-org/tinygo/goenv" ) +var gorootCreateMutex sync.Mutex + // GetCachedGoroot creates a new GOROOT by merging both the standard GOROOT and // the GOROOT from TinyGo using lots of symbolic links. func GetCachedGoroot(config *compileopts.Config) (string, error) { @@ -54,6 +57,15 @@ func GetCachedGoroot(config *compileopts.Config) (string, error) { cachedgoroot += "-syscall" } + // Do not try to create the cached GOROOT in parallel, that's only a waste + // of I/O bandwidth and thus speed. Instead, use a mutex to make sure only + // one goroutine does it at a time. + // This is not a way to ensure atomicity (a different TinyGo invocation + // could be creating the same directory), but instead a way to avoid + // creating it many times in parallel when running tests in parallel. + gorootCreateMutex.Lock() + defer gorootCreateMutex.Unlock() + if _, err := os.Stat(cachedgoroot); err == nil { return cachedgoroot, nil } @@ -88,6 +100,15 @@ func GetCachedGoroot(config *compileopts.Config) (string, error) { // deleted by the defer above. return cachedgoroot, nil } + if runtime.GOOS == "windows" && os.IsPermission(err) { + // On Windows, a rename with a destination directory that already + // exists does not result in an IsExist error, but rather in an + // access denied error. To be sure, check for this case by checking + // whether the target directory exists. + if _, err := os.Stat(cachedgoroot); err == nil { + return cachedgoroot, nil + } + } return "", err } return cachedgoroot, nil