search

tetratelabs / wazero

wazero: the zero dependency WebAssembly runtime for Go developers
https://wazero.io
Apache License 2.0
4.85k stars 253 forks source link

Data race in Memory.Grow with concurrency #2278

Closed anuraaga closed 3 months ago

anuraaga commented 3 months ago

Describe the bug A clear and concise description of what the bug is.

Go race detector finds data races within Grow in concurrent applications using Wasm threads proposal.

We know that the guest locks around Grow and don't believe there is any thread-unsafety happening. However, this is in compiled code so the Go race detector can't understand it, and I guess it should be a priority to allow downstream users of wazero to run race detector on their apps without warnings. I wonder if we should go ahead and just lock around Grow when using shared memory - does that sound reasonable?

To Reproduce Description of the host and wasm code that reproduces the behavior. Smoothest debugging will be if you can share a repository with the actual code.

go test -race . in go-re2. It reproduces flakily.

Expected behavior A clear and concise description of what you expected to happen.

No data races in Go apps using wazero

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the relevant information):

Additional context Add any other context about the problem here.

==================
WARNING: DATA RACE
Write at 0x00c00015839c by goroutine 98:
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:249 +0x19c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Previous write at 0x00c00015839c by goroutine 106:
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:249 +0x19c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 98 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 106 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c00001629f by goroutine 98:
  encoding/binary.littleEndian.PutUint64()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/encoding/binary/binary.go:112 +0x14c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).putLocalMemory()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:235 +0xdc
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).MemoryGrown()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:222 +0x3c
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:270 +0x390
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Previous write at 0x00c00001629f by goroutine 106:
  encoding/binary.littleEndian.PutUint64()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/encoding/binary/binary.go:120 +0x200
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).putLocalMemory()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:235 +0xdc
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).MemoryGrown()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:222 +0x3c
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:270 +0x390
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 98 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 106 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c0000162a7 by goroutine 98:
  encoding/binary.littleEndian.PutUint64()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/encoding/binary/binary.go:112 +0x268
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).putLocalMemory()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:236 +0x138
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).MemoryGrown()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:222 +0x3c
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:270 +0x390
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Previous write at 0x00c0000162a7 by goroutine 106:
  encoding/binary.littleEndian.PutUint64()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/encoding/binary/binary.go:120 +0x31c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).putLocalMemory()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:236 +0x138
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*moduleEngine).MemoryGrown()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/module_engine.go:222 +0x3c
  github.com/tetratelabs/wazero/internal/wasm.(*MemoryInstance).Grow()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/wasm/memory.go:270 +0x390
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).callWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:308 +0x59c
  github.com/tetratelabs/wazero/internal/engine/wazevo.(*callEngine).CallWithStack()
      /Users/anuraag/go/pkg/mod/github.com/tetratelabs/wazero@v1.7.3/internal/engine/wazevo/call_engine.go:191 +0x110
  github.com/wasilibs/go-re2/internal.(*lazyFunction).callWithStack()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:593 +0x1d4
  github.com/wasilibs/go-re2/internal.(*lazyFunction).Call1()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:552 +0x70
  github.com/wasilibs/go-re2/internal.malloc()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:451 +0x4c
  github.com/wasilibs/go-re2/internal.(*libre2ABI).reserve()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:472 +0x88
  github.com/wasilibs/go-re2/internal.(*libre2ABI).startOperation()
      /Users/anuraag/git/go-re2/internal/re2_wazero.go:223 +0x7c
  github.com/wasilibs/go-re2/internal.(*Regexp).FindSubmatchIndex()
      /Users/anuraag/git/go-re2/internal/re2.go:568 +0x40
  github.com/wasilibs/go-re2.TestHeavyGC.func2()
      /Users/anuraag/git/go-re2/stress_test.go:78 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 98 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40

Goroutine 106 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x5e4
  github.com/wasilibs/go-re2.TestHeavyGC()
      /Users/anuraag/git/go-re2/stress_test.go:74 +0xf4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1689 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.22.4/libexec/src/testing/testing.go:1742 +0x40
mathetake commented 3 months ago

I wonder if we should go ahead and just lock around Grow when using shared memory - does that sound reasonable?

sounds good to me