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.

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

Race in txpool tests #8409

Closed
AskAlexSharov opened this issue Oct 9, 2023 · 2 comments
Closed

Race in txpool tests #8409

AskAlexSharov opened this issue Oct 9, 2023 · 2 comments
Assignees

Comments

@AskAlexSharov
Copy link
Collaborator

go test  --timeout 30m -tags=integration,nosqlite,noboltdb -race -run='TestSendRawTransaction' -v -count 10 ./turbo/jsonrpc
==================
WARNING: DATA RACE
Write at 0x00c004894b38 by goroutine 9067:
  runtime.racewrite()
      <autogenerated>:1 +0x1e
  github.com/ledgerwatch/erigon-lib/kv/mdbx.(*MdbxKV).Close()
      github.com/ledgerwatch/erigon-lib@v1.0.0/kv/mdbx/kv_mdbx.go:466 +0x6b
  github.com/ledgerwatch/erigon/turbo/stages/mock.(*MockSentry).Close()
      github.com/ledgerwatch/erigon/turbo/stages/mock/mock_sentry.go:120 +0x90
  github.com/ledgerwatch/erigon/turbo/stages/mock.(*MockSentry).Close-fm()
      <autogenerated>:1 +0x33
  testing.(*common).Cleanup.func1()
      testing/testing.go:1169 +0x182
  testing.(*common).runCleanup()
      testing/testing.go:1347 +0x1c2
  testing.tRunner.func2()
      testing/testing.go:1589 +0x50
  runtime.deferreturn()
      runtime/panic.go:477 +0x30
  testing.(*T).Run.func1()
      testing/testing.go:1648 +0x44

Previous read at 0x00c004894b38 by goroutine 9068:
  runtime.raceread()
      <autogenerated>:1 +0x1e
  github.com/ledgerwatch/erigon-lib/kv/mdbx.(*MdbxKV).BeginRo()
      github.com/ledgerwatch/erigon-lib@v1.0.0/kv/mdbx/kv_mdbx.go:508 +0x448
  github.com/ledgerwatch/erigon-lib/txpool.(*Fetch).handleStateChanges()
      github.com/ledgerwatch/erigon-lib@v1.0.0/txpool/fetch.go:439 +0x20a
  github.com/ledgerwatch/erigon-lib/txpool.(*Fetch).ConnectCore.func1()
      github.com/ledgerwatch/erigon-lib@v1.0.0/txpool/fetch.go:120 +0xa4

Goroutine 9067 (running) created at:
  testing.(*T).Run()
      testing/testing.go:1648 +0x82a
  testing.runTests.func1()
      testing/testing.go:2054 +0x84
  testing.tRunner()
      testing/testing.go:1595 +0x238
  testing.runTests()
      testing/testing.go:2052 +0x896
  testing.(*M).Run()
      testing/testing.go:1925 +0xb57
  main.main()
      _testmain.go:267 +0x2bd

Goroutine 9068 (running) created at:
  github.com/ledgerwatch/erigon-lib/txpool.(*Fetch).ConnectCore()
      github.com/ledgerwatch/erigon-lib@v1.0.0/txpool/fetch.go:113 +0x8d
  github.com/ledgerwatch/erigon/turbo/stages/mock.MockWithEverything()
      github.com/ledgerwatch/erigon/turbo/stages/mock/mock_sentry.go:325 +0x2b59
  github.com/ledgerwatch/erigon/turbo/stages/mock.MockWithTxPool()
      github.com/ledgerwatch/erigon/turbo/stages/mock/mock_sentry.go:542 +0x4f9
  github.com/ledgerwatch/erigon/turbo/jsonrpc_test.TestSendRawTransaction()
      github.com/ledgerwatch/erigon/turbo/jsonrpc/send_transaction_test.go:79 +0x3e
  testing.tRunner()
      testing/testing.go:1595 +0x238
  testing.(*T).Run.func1()
      testing/testing.go:1648 +0x44
==================
    testing.go:1465: race detected during execution of test
@AskAlexSharov AskAlexSharov changed the title Race in tests Race in txpool tests Oct 9, 2023
@battlmonstr battlmonstr self-assigned this Jan 16, 2024
battlmonstr added a commit that referenced this issue Jan 16, 2024
In the previous code WaitGroup db.wg.Add(), Wait() and db.closed were not treated in sync.
In particular, it was theoretically possible to first check closed, then set closed and Wait, and then call wg.Add() while waiting
(leading to WaitGroup panic).
In theory it was also possible that db.env.BeginTxn() is called on a closed or nil db.env,
because db.wg.Add() was called only after BeginTxn (db.wg.Wait() could already return).

WaitGroup is replaced with a Cond variable.
Now it is not possible to increase the active transactions count on a closed database.
It is also not possible to call BeginTxn on a closed database.
battlmonstr added a commit that referenced this issue Jan 17, 2024
In the previous code WaitGroup db.wg.Add(), Wait() and db.closed were not treated in sync.
In particular, it was theoretically possible to first check closed, then set closed and Wait, and then call wg.Add() while waiting
(leading to WaitGroup panic).
In theory it was also possible that db.env.BeginTxn() is called on a closed or nil db.env,
because db.wg.Add() was called only after BeginTxn (db.wg.Wait() could already return).

WaitGroup is replaced with a Cond variable.
Now it is not possible to increase the active transactions count on a closed database.
It is also not possible to call BeginTxn on a closed database.
battlmonstr added a commit that referenced this issue Jan 17, 2024
In the previous code WaitGroup db.wg.Add(), Wait() and db.closed were
not treated in sync. In particular, it was theoretically possible to
first check closed, then set closed and Wait, and then call wg.Add()
while waiting (leading to WaitGroup panic).
In theory it was also possible that db.env.BeginTxn() is called on a
closed or nil db.env, because db.wg.Add() was called only after BeginTxn
(db.wg.Wait() could already return).

WaitGroup is replaced with a Cond variable.
Now it is not possible to increase the active transactions count on a
closed database. It is also not possible to call BeginTxn on a closed
database.
battlmonstr added a commit that referenced this issue Jan 17, 2024
In the previous code WaitGroup db.wg.Add(), Wait() and db.closed were
not treated in sync. In particular, it was theoretically possible to
first check closed, then set closed and Wait, and then call wg.Add()
while waiting (leading to WaitGroup panic).
In theory it was also possible that db.env.BeginTxn() is called on a
closed or nil db.env, because db.wg.Add() was called only after BeginTxn
(db.wg.Wait() could already return).

WaitGroup is replaced with a Cond variable.
Now it is not possible to increase the active transactions count on a
closed database. It is also not possible to call BeginTxn on a closed
database.
@yperbasis
Copy link
Member

@AskAlexSharov @battlmonstr is this issue fixed?

@AskAlexSharov
Copy link
Collaborator Author

yes. thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants