Skip to content

Commit

Permalink
fix #878: concurrent map write with inject
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 24, 2021
1 parent f5e0032 commit 5660fcd
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Fix a concurrent map write with the `--inject:` feature ([#878](https://github.com/evanw/esbuild/issues/878))

This release fixes an issue where esbuild could potentially crash sometimes with a concurrent map write when using injected files and entry points that were neither relative nor absolute paths. This was an edge case where esbuild's low-level file subsystem was being used without being behind a mutex lock. This regression was likely introduced in version 0.8.21. The cause of the crash has been fixed.

## 0.8.51

* The stderr log format now contains line numbers after file names ([#865](https://github.com/evanw/esbuild/issues/865))
Expand Down
13 changes: 11 additions & 2 deletions internal/fs/fs_real.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import (

type realFS struct {
// Stores the file entries for directories we've listed before
entries map[string]entriesOrErr
entriesMutex sync.Mutex
entries map[string]entriesOrErr

// If true, do not use the "entries" cache
doNotCacheEntries bool
Expand Down Expand Up @@ -117,7 +118,13 @@ func RealFS(options RealFSOptions) (FS, error) {
func (fs *realFS) ReadDirectory(dir string) (DirEntries, error) {
if !fs.doNotCacheEntries {
// First, check the cache
if cached, ok := fs.entries[dir]; ok {
cached, ok := func() (cached entriesOrErr, ok bool) {
fs.entriesMutex.Lock()
defer fs.entriesMutex.Unlock()
cached, ok = fs.entries[dir]
return
}()
if ok {
// Cache hit: stop now
return cached.entries, cached.err
}
Expand Down Expand Up @@ -166,6 +173,8 @@ func (fs *realFS) ReadDirectory(dir string) (DirEntries, error) {
entries.data = nil
}
if !fs.doNotCacheEntries {
fs.entriesMutex.Lock()
defer fs.entriesMutex.Unlock()
fs.entries[dir] = entriesOrErr{entries: entries, err: err}
}
return entries, err
Expand Down
11 changes: 10 additions & 1 deletion internal/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ type resolver struct {
log logger.Log
caches *cache.CacheSet
options config.Options
mutex sync.Mutex

// A special filtered import order for CSS "@import" imports.
//
Expand All @@ -152,6 +151,16 @@ type resolver struct {
// picture but it's better than some alternatives and probably pretty good.
atImportExtensionOrder []string

// This mutex serves two purposes. First of all, it guards access to "dirCache"
// which is potentially mutated during path resolution. But this mutex is also
// necessary for performance. The "React admin" benchmark mysteriously runs
// twice as fast when this mutex is locked around the whole resolve operation
// instead of around individual accesses to "dirCache". For some reason,
// reducing parallelism in the resolver helps the rest of the bundler go
// faster. I'm not sure why this is but please don't change this unless you
// do a lot of testing with various benchmarks and there aren't any regressions.
mutex sync.Mutex

// This cache maps a directory path to information about that directory and
// all parent directories
dirCache map[string]*dirInfo
Expand Down

0 comments on commit 5660fcd

Please sign in to comment.