diff --git a/.github/workflows/commit.yaml b/.github/workflows/commit.yaml index f584a34a..e5027722 100644 --- a/.github/workflows/commit.yaml +++ b/.github/workflows/commit.yaml @@ -76,6 +76,11 @@ jobs: cache: true - run: make test + if: ${{ github.event_name == 'pull_request' }} + + # Run tests with -race only on main branch push. + - run: make test go_test_options='-race' + if: ${{ github.event_name == 'push' }} - name: "Generate coverage report" # only once (not per OS) if: runner.os == 'Linux' diff --git a/Makefile b/Makefile index 606369d3..46dbd96c 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,9 @@ main_sources := $(wildcard $(filter-out %_test.go $(all_testdata) $(all_testing # Paths need to all start with ./, so we do that manually vs foreach which strips it. main_packages := $(sort $(foreach f,$(dir $(main_sources)),$(if $(findstring ./,$(f)),./,./$(f)))) +# By default, we don't run with -race as it's costly to run on every PR. +go_test_options ?= + ensureCompilerFastest := -ldflags '-X github.com/tetratelabs/wazero/internal/integration_test/vs.ensureCompilerFastest=true' .PHONY: bench bench: @@ -45,7 +48,7 @@ build.bench: .PHONY: test.examples test.examples: - @go test ./examples/... ./imports/assemblyscript/example/... ./imports/emscripten/... ./imports/go/example/... ./imports/wasi_snapshot_preview1/example/... + @go test $(go_test_options) ./examples/... ./imports/assemblyscript/example/... ./imports/emscripten/... ./imports/go/example/... ./imports/wasi_snapshot_preview1/example/... .PHONY: build.examples.as build.examples.as: @@ -174,8 +177,8 @@ build.spectest.v2: # Note: SIMD cases are placed in the "simd" subdirectory. .PHONY: test test: - @go test $$(go list ./... | grep -vE '$(spectest_v1_dir)|$(spectest_v2_dir)') -timeout 120s - @cd internal/version/testdata && go test ./... -timeout 120s + @go test $(go_test_options) $$(go list ./... | grep -vE '$(spectest_v1_dir)|$(spectest_v2_dir)') -timeout 120s + @cd internal/version/testdata && go test $(go_test_options) ./... -timeout 120s .PHONY: coverage coverpkg = $(subst $(space),$(comma),$(main_packages)) @@ -189,10 +192,10 @@ spectest: @$(MAKE) spectest.v2 spectest.v1: - @go test $$(go list ./... | grep $(spectest_v1_dir)) -timeout 120s + @go test $(go_test_options) $$(go list ./... | grep $(spectest_v1_dir)) -timeout 120s spectest.v2: - @go test $$(go list ./... | grep $(spectest_v2_dir)) -timeout 120s + @go test $(go_test_options) $$(go list ./... | grep $(spectest_v2_dir)) -timeout 120s golangci_lint_path := $(shell go env GOPATH)/bin/golangci-lint diff --git a/cache.go b/cache.go index 1cb91209..34bb53d6 100644 --- a/cache.go +++ b/cache.go @@ -8,6 +8,7 @@ import ( "path" "path/filepath" goruntime "runtime" + "sync" "github.com/tetratelabs/wazero/api" "github.com/tetratelabs/wazero/internal/filecache" @@ -50,9 +51,14 @@ func NewCompilationCacheWithDir(dirname string) (CompilationCache, error) { type cache struct { // eng is the engine for this cache. If the cache is configured, the engine is shared across multiple instances of // Runtime, and its lifetime is not bound to them. Instead, the engine is alive until Cache.Close is called. - eng wasm.Engine - + eng wasm.Engine fileCache filecache.Cache + once sync.Once +} + +func (c *cache) initEngine(ne newEngine, ctx context.Context, features api.CoreFeatures) wasm.Engine { + c.once.Do(func() { c.eng = ne(ctx, features, c.fileCache) }) + return c.eng } // Close implements the same method on the Cache interface. diff --git a/config.go b/config.go index 88f67159..a8c4bc4a 100644 --- a/config.go +++ b/config.go @@ -125,13 +125,15 @@ func NewRuntimeConfig() RuntimeConfig { return newRuntimeConfig() } +type newEngine func(context.Context, api.CoreFeatures, filecache.Cache) wasm.Engine + type runtimeConfig struct { enabledFeatures api.CoreFeatures memoryLimitPages uint32 memoryCapacityFromMax bool isInterpreter bool dwarfDisabled bool // negative as defaults to enabled - newEngine func(context.Context, api.CoreFeatures, filecache.Cache) wasm.Engine + newEngine newEngine cache CompilationCache } diff --git a/runtime.go b/runtime.go index 06284b39..2634886c 100644 --- a/runtime.go +++ b/runtime.go @@ -116,10 +116,7 @@ func NewRuntimeWithConfig(ctx context.Context, rConfig RuntimeConfig) Runtime { if c := config.cache; c != nil { // If the Cache is configured, we share the engine. cacheImpl = c.(*cache) - if cacheImpl.eng == nil { - cacheImpl.eng = config.newEngine(ctx, config.enabledFeatures, cacheImpl.fileCache) - } - engine = cacheImpl.eng + engine = cacheImpl.initEngine(config.newEngine, ctx, config.enabledFeatures) } else { // Otherwise, we create a new engine. engine = config.newEngine(ctx, config.enabledFeatures, nil) diff --git a/runtime_test.go b/runtime_test.go index 458a1f31..fbd91a2f 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -4,6 +4,7 @@ import ( "context" _ "embed" "errors" + "sync" "testing" "time" @@ -695,3 +696,21 @@ func (e *mockEngine) Close() (err error) { e.closed = true return } + +// TestNewRuntime_concurrent ensures that concurrent execution of NewRuntime is race-free. +// This depends on -race flag. +func TestNewRuntime_concurrent(t *testing.T) { + const num = 100 + var wg sync.WaitGroup + config := NewRuntimeConfig().WithCompilationCache(NewCompilationCache()) + wg.Add(num) + for i := 0; i < num; i++ { + go func() { + defer wg.Done() + r := NewRuntimeWithConfig(testCtx, config) + err := r.Close(testCtx) + require.NoError(t, err) + }() + } + wg.Wait() +}