Skip to content

Commit

Permalink
loader: be more robust when creating the cached GOROOT
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aykevl authored and deadprogram committed Aug 28, 2020
1 parent ae01904 commit ecaf946
Showing 1 changed file with 21 additions and 0 deletions.
21 changes: 21 additions & 0 deletions loader/goroot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ecaf946

Please sign in to comment.