Skip to content

Commit ecaf946

Browse files
aykevldeadprogram
authored andcommitted
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.
1 parent ae01904 commit ecaf946

File tree

1 file changed

+21
-0
lines changed

1 file changed

+21
-0
lines changed

loader/goroot.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,14 @@ import (
1515
"path"
1616
"path/filepath"
1717
"runtime"
18+
"sync"
1819

1920
"github.com/tinygo-org/tinygo/compileopts"
2021
"github.com/tinygo-org/tinygo/goenv"
2122
)
2223

24+
var gorootCreateMutex sync.Mutex
25+
2326
// GetCachedGoroot creates a new GOROOT by merging both the standard GOROOT and
2427
// the GOROOT from TinyGo using lots of symbolic links.
2528
func GetCachedGoroot(config *compileopts.Config) (string, error) {
@@ -54,6 +57,15 @@ func GetCachedGoroot(config *compileopts.Config) (string, error) {
5457
cachedgoroot += "-syscall"
5558
}
5659

60+
// Do not try to create the cached GOROOT in parallel, that's only a waste
61+
// of I/O bandwidth and thus speed. Instead, use a mutex to make sure only
62+
// one goroutine does it at a time.
63+
// This is not a way to ensure atomicity (a different TinyGo invocation
64+
// could be creating the same directory), but instead a way to avoid
65+
// creating it many times in parallel when running tests in parallel.
66+
gorootCreateMutex.Lock()
67+
defer gorootCreateMutex.Unlock()
68+
5769
if _, err := os.Stat(cachedgoroot); err == nil {
5870
return cachedgoroot, nil
5971
}
@@ -88,6 +100,15 @@ func GetCachedGoroot(config *compileopts.Config) (string, error) {
88100
// deleted by the defer above.
89101
return cachedgoroot, nil
90102
}
103+
if runtime.GOOS == "windows" && os.IsPermission(err) {
104+
// On Windows, a rename with a destination directory that already
105+
// exists does not result in an IsExist error, but rather in an
106+
// access denied error. To be sure, check for this case by checking
107+
// whether the target directory exists.
108+
if _, err := os.Stat(cachedgoroot); err == nil {
109+
return cachedgoroot, nil
110+
}
111+
}
91112
return "", err
92113
}
93114
return cachedgoroot, nil

0 commit comments

Comments
 (0)