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

mssmt: fix unit test data race #140

Merged
merged 1 commit into from
Sep 27, 2022
Merged

mssmt: fix unit test data race #140

merged 1 commit into from
Sep 27, 2022

Conversation

bhandras
Copy link
Member

Roasbeef
Roasbeef approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🦊

@Roasbeef Roasbeef merged commit e72a61d into lightninglabs:main Sep 27, 2022
@Roasbeef
Copy link
Member

I think we might have another case (could be the test iself tho):

==================
WARNING: DATA RACE
Read at 0x00c00044dc20 by goroutine 87:
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeHash()
      /home/runner/work/taro/taro/mssmt/node.go:210 +0x5b
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:164 +0x66
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:1[27](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:28) +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:131 +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Previous write at 0x00c00044dc20 by goroutine 90:
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeHash()
      /home/runner/work/taro/taro/mssmt/node.go:221 +0x1e8
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:164 +0x66
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:127 +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:131 +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 87 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 90 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47
==================
==================
WARNING: DATA RACE
Read at 0x00c000432020 by goroutine 87:
  runtime.racereadrange()
      <autogenerated>:1 +0x1b
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:164 +0x66
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:127 +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:131 +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Previous write at 0x00c000432020 by goroutine 90:
  runtime.slicecopy()
      /opt/hostedtoolcache/go/1.18.4/x64/src/runtime/slice.go:295 +0x0
  crypto/sha256.(*digest).Sum()
      /opt/hostedtoolcache/go/1.18.4/x64/src/crypto/sha256/sha256.go:214 +0x224
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeHash()
      /home/runner/work/taro/taro/mssmt/node.go:221 +0x1c9
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:164 +0x66
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:127 +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:131 +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 87 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 90 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47
==================
==================
WARNING: DATA RACE
Read at 0x00c00044dc[28](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:29) by goroutine 87:
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeSum()
      /home/runner/work/taro/taro/mssmt/node.go:227 +0x3b
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:172 +0x1d1
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:127 +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:131 +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Previous write at 0x00c00044dc28 by goroutine 90:
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeSum()
      /home/runner/work/taro/taro/mssmt/node.go:232 +0xd8
  github.com/lightninglabs/taro/mssmt.(*BranchNode).NodeHash()
      /home/runner/work/taro/taro/mssmt/node.go:220 +0x16b
  github.com/lightninglabs/taro/tarodb.(*taroTreeStoreTx).InsertBranch()
      /home/runner/work/taro/taro/tarodb/mssmt.go:164 +0x66
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1.1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:173 +0x332
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update.func1()
      /home/runner/work/taro/taro/tarodb/mssmt.go:127 +0x155
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      /home/runner/work/taro/taro/tarodb/interfaces.go:118 +0x16e
  github.com/lightninglabs/taro/tarodb.(*TransactionExecutor[...]).ExecTx()
      <autogenerated>:1 +0x86
  github.com/lightninglabs/taro/tarodb.(*TaroTreeStore).Update()
      /home/runner/work/taro/taro/tarodb/mssmt.go:1[31](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:32) +0x183
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion.func1()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:153 +0x3a5
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:14[39](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:40) +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 87 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1[43](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:44)9 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x47

Goroutine 90 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x724
  github.com/lightninglabs/taro/tarodb.TestTreeDeletion()
      /home/runner/work/taro/taro/tarodb/mssmt_test.go:148 +0x12c8
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.4/x64/src/testing/testing.go:1486 +0x[47](https://github.com/lightninglabs/taro/actions/runs/3139942137/jobs/5100831234#step:5:48)

@Roasbeef Roasbeef mentioned this pull request Sep 28, 2022
Closed
Roasbeef
Roasbeef reviewed Sep 28, 2022
View reviewed changes
mssmt/tree.go
@@ -35,6 +35,7 @@ func init() {
// read/populate this value causing a race condition.
branch := NewBranch(EmptyTree[i+1], EmptyTree[i+1])
branch.NodeHash()
branch.NodeSum()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again, the call to NodeHash should already be calling NodeSum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Roasbeef Roasbeef Roasbeef approved these changes

@jharveyb jharveyb Awaiting requested review from jharveyb

Assignees
No one assigned
Labels
None yet
Projects
Taproot-Assets Project Board
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bhandras @Roasbeef