Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Data race in Memory.Grow with concurrency #2278

Closed
anuraaga opened this issue Jul 3, 2024 · 1 comment · Fixed by #2279
Closed

Data race in Memory.Grow with concurrency #2278

anuraaga opened this issue Jul 3, 2024 · 1 comment · Fixed by #2279
Assignees
@anuraaga
Labels
bug Something isn't working

Comments

@anuraaga
Copy link
Contributor

anuraaga commented Jul 3, 2024 •
edited
Loading

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):

  • Go version: 1.22.4
  • wazero Version: 1.7.3
  • Host architecture: arm64
  • Runtime mode: compiler

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
@anuraaga anuraaga added the bug Something isn't working label Jul 3, 2024
@anuraaga anuraaga mentioned this issue Jul 3, 2024
Closed
@mathetake
Copy link
Member

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

sounds good to me

@anuraaga anuraaga mentioned this issue Jul 4, 2024
Merged
mathetake pushed a commit that referenced this issue Jul 4, 2024
@anuraaga
threads: lock around invocations of memory.Grow (#2279)
1734cdb 
fixes #2278

Signed-off-by: Anuraag Agrawal <anuraaga@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@anuraaga anuraaga

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

threads: lock around invocations of memory.Grow anuraaga/wazero
2 participants
@anuraaga @mathetake