rawdb: Race condition in freezer table

[MariusVanDerWijden]

Hi I noticed a race condition between
func (t *freezerTable) Retrieve(item uint64) ([]byte, error) { and
func (t *freezerTable) Close() error {
because retrieve does not lock the rlock at the beginning.
We could use the same methodology as in

go-ethereum/core/rawdb/freezer_table.go

Line 459 in c2d65d3

t.lock.RLock()

However I think since close isn't called often we could just not acquire the second lock as it would impact every retrieve calls performance.
If you guys agree, please put a wontfix label and close the issue!

Stack trace

==================
WARNING: DATA RACE
Write at 0x00c0005b3420 by goroutine 47:
  github.com/ethereum/go-ethereum/core/rawdb.(*freezerTable).Close()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/freezer_table.go:402 +0x3ec
  github.com/ethereum/go-ethereum/core/rawdb.(*freezer).Close()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/freezer.go:131 +0x153
  github.com/ethereum/go-ethereum/core/rawdb.(*freezerdb).Close()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/database.go:47 +0xc5
  github.com/ethereum/go-ethereum/eth.(*Ethereum).Stop()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/eth/backend.go:561 +0x182
  github.com/ethereum/go-ethereum/node.(*Node).Stop()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/node.go:447 +0x307
  github.com/ethereum/go-ethereum/node.(*Node).Close()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/node.go:131 +0x50
  github.com/ethereum/go-ethereum/console.(*tester).Close()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:155 +0xe5
  github.com/ethereum/go-ethereum/console.TestEvaluate()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:197 +0x152
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).checkAndCleanFiles()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db_util.go:52 +0x2f8
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:136 +0x9f0
  fmt.(*ss).doScanf()
      /snap/go/4762/src/fmt/scan.go:1217 +0x382
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).recoverJournal()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:482 +0xc5
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:131 +0x9cc
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:203 +0x1d3
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*session).recover.func1()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:134 +0xf4
  github.com/syndtr/goleveldb/leveldb.(*session).recover()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:142 +0x1938
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:189 +0xf8
  github.com/syndtr/goleveldb/leveldb.OpenFile()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:225 +0xa5
  github.com/ethereum/go-ethereum/p2p/enode.newPersistentDB()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/p2p/enode/nodedb.go:95 +0xd2
  github.com/ethereum/go-ethereum/p2p/enode.OpenDB()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/p2p/enode/nodedb.go:79 +0x9a
  github.com/ethereum/go-ethereum/p2p.(*Server).setupLocalNode()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/p2p/server.go:498 +0x561
  github.com/ethereum/go-ethereum/p2p.(*Server).Start()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/p2p/server.go:469 +0x744
  github.com/ethereum/go-ethereum/node.(*Node).Start()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/node.go:220 +0xeb4
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).checkAndCleanFiles()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db_util.go:52 +0x2f8
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:136 +0x9f0
  fmt.(*ss).doScanf()
      /snap/go/4762/src/fmt/scan.go:1217 +0x382
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).recoverJournal()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:482 +0xc5
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:131 +0x9cc
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:203 +0x1d3
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*session).recover.func1()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:134 +0xf4
  github.com/syndtr/goleveldb/leveldb.(*session).recover()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:142 +0x1938
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:189 +0xf8
  github.com/syndtr/goleveldb/leveldb.OpenFile()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:225 +0xa5
  github.com/ethereum/go-ethereum/ethdb/leveldb.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/ethdb/leveldb/leveldb.go:98 +0x3be
  github.com/ethereum/go-ethereum/core/rawdb.NewLevelDBDatabaseWithFreezer()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/database.go:210 +0x83
  github.com/ethereum/go-ethereum/node.(*ServiceContext).OpenDatabaseWithFreezer()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/service.go:68 +0x319
  github.com/ethereum/go-ethereum/eth.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/eth/backend.go:134 +0x432
  github.com/ethereum/go-ethereum/console.newTester.func1()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:111 +0x4f
  github.com/ethereum/go-ethereum/node.(*Node).Start()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/node.go:206 +0x73d
  github.com/ethereum/go-ethereum/console.newTester()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:115 +0x503
  github.com/ethereum/go-ethereum/console.TestEvaluate()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:190 +0x59
  testing.tRunner()
      /snap/go/4762/src/testing/testing.go:909 +0x199

Previous read at 0x00c0005b3420 by goroutine 156:
  github.com/ethereum/go-ethereum/core/rawdb.(*freezerTable).Retrieve()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/freezer_table.go:545 +0x6e3
  github.com/ethereum/go-ethereum/core/rawdb.(*freezer).Ancient()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/freezer.go:156 +0xb4
  github.com/ethereum/go-ethereum/core/rawdb.(*freezerdb).Ancient()
      <autogenerated>:1 +0x98
  github.com/ethereum/go-ethereum/core/rawdb.ReadHeaderRLP()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/accessors_chain.go:184 +0x1a5
  github.com/ethereum/go-ethereum/core/rawdb.ReadHeader()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/accessors_chain.go:203 +0xa1
  github.com/ethereum/go-ethereum/core/rawdb.ReadBlock()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/accessors_chain.go:470 +0xa1
  github.com/ethereum/go-ethereum/core.(*BlockChain).GetBlock()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/blockchain.go:728 +0x1ac
  github.com/ethereum/go-ethereum/core.(*BlockChain).GetBlocksFromHash()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/blockchain.go:781 +0x22a
  github.com/ethereum/go-ethereum/miner.(*worker).makeCurrent()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/miner/worker.go:638 +0x4f1
  github.com/ethereum/go-ethereum/miner.(*worker).commitNewWork()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/miner/worker.go:883 +0x7a8
  github.com/ethereum/go-ethereum/miner.(*worker).mainLoop()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/miner/worker.go:408 +0x1403

Goroutine 47 (running) created at:
  testing.(*T).Run()
      /snap/go/4762/src/testing/testing.go:960 +0x651
  testing.runTests.func1()
      /snap/go/4762/src/testing/testing.go:1202 +0xa6
  testing.tRunner()
      /snap/go/4762/src/testing/testing.go:909 +0x199
  testing.runTests()
      /snap/go/4762/src/testing/testing.go:1200 +0x521
  testing.(*M).Run()
      /snap/go/4762/src/testing/testing.go:1117 +0x2ff
  main.main()
      _testmain.go:58 +0x223

Goroutine 156 (finished) created at:
  github.com/ethereum/go-ethereum/miner.newWorker()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/miner/worker.go:216 +0x9e8
  github.com/ethereum/go-ethereum/miner.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/miner/miner.go:75 +0xbd
  github.com/ethereum/go-ethereum/eth.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/eth/backend.go:213 +0x1719
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).checkAndCleanFiles()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db_util.go:52 +0x2f8
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:136 +0x9f0
  fmt.(*ss).doScanf()
      /snap/go/4762/src/fmt/scan.go:1217 +0x382
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*DB).recoverJournal()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:482 +0xc5
  github.com/syndtr/goleveldb/leveldb.openDB()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:131 +0x9cc
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:203 +0x1d3
  fmt.Fscanf()
      /snap/go/4762/src/fmt/scan.go:143 +0xcf
  github.com/syndtr/goleveldb/leveldb/storage.fsParseName()
      /snap/go/4762/src/fmt/scan.go:114 +0x192
  github.com/syndtr/goleveldb/leveldb/storage.(*fileStorage).List()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/storage/file_storage.go:458 +0x2c4
  github.com/syndtr/goleveldb/leveldb.(*session).recover.func1()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:134 +0xf4
  github.com/syndtr/goleveldb/leveldb.(*session).recover()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/session.go:142 +0x1938
  github.com/syndtr/goleveldb/leveldb.Open()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:189 +0xf8
  github.com/syndtr/goleveldb/leveldb.OpenFile()
      /home/asdf/go/pkg/mod/github.com/syndtr/goleveldb@v1.0.1-0.20190923125748-758128399b1d/leveldb/db.go:225 +0xa5
  github.com/ethereum/go-ethereum/ethdb/leveldb.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/ethdb/leveldb/leveldb.go:98 +0x3be
  github.com/ethereum/go-ethereum/core/rawdb.NewLevelDBDatabaseWithFreezer()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/core/rawdb/database.go:210 +0x83
  github.com/ethereum/go-ethereum/node.(*ServiceContext).OpenDatabaseWithFreezer()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/service.go:68 +0x319
  github.com/ethereum/go-ethereum/eth.New()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/eth/backend.go:134 +0x432
  github.com/ethereum/go-ethereum/console.newTester.func1()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:111 +0x4f
  github.com/ethereum/go-ethereum/node.(*Node).Start()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/node/node.go:206 +0x73d
  github.com/ethereum/go-ethereum/console.newTester()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:115 +0x503
  github.com/ethereum/go-ethereum/console.TestEvaluate()
      /home/asdf/go/src/github.com/ethereum/go-ethereum/console/console_test.go:190 +0x59
  testing.tRunner()
      /snap/go/4762/src/testing/testing.go:909 +0x199
==================
--- FAIL: TestEvaluate (0.69s)
    testing.go:853: race detected during execution of test
FAIL

[mvdan]

I've encountered the very same issue when using https://godoc.org/github.com/ethereum/go-ethereum/node#Node.Stop on a node that was started earlier via https://godoc.org/github.com/ethereum/go-ethereum/node#Node.Start. The race detector trace looks almost exactly the same, involving freezerTable.Close.

@karalabe would a patch here be welcome? Any tips or thoughts?

[karalabe]

@mvdan Patches are always welcome :)

The issue definitely is that head and index are accessed out-of-lock in Retrieve, which Close modifies. I think it's reasonable to move this lock https://github.com/ethereum/go-ethereum/blob/master/core/rawdb/freezer_table.go#L556 up to the head of the function. Just make sure all error paths then properly unlock it.

We could try to be even more optimal by doing the atomic checks first and only then checking the head and index existence, but honestly that seems super overkill. The general case is that we are trying to access existing data so we will realistically (almost) never hit out of bound cases that could avoid the locking.

Just to make sure I didn't miss anything, cc @holiman