[Bug]: race condition between `baseapp.Commit` and `baseapp.CreateQueryContext `

[beer-1]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

We found race condition rs.lastCommitInfo between baseapp.CreateQueryContext and baseapp.Commit.

case 1.

2024-11-26T07:47:34.3846975Z ==================
2024-11-26T07:47:34.3847765Z WARNING: DATA RACE
2024-11-26T07:47:34.3848509Z Read at 0x00c091570220 by goroutine 4277:
2024-11-26T07:47:34.3849460Z   cosmossdk.io/store/rootmulti.(*Store).LastCommitID()
2024-11-26T07:47:34.3855278Z       /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:441 +0x4c
2024-11-26T07:47:34.3856100Z   cosmossdk.io/store/rootmulti.(*Store).LatestVersion()
2024-11-26T07:47:34.3856999Z       /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:436 +0x26
2024-11-26T07:47:34.3858198Z   github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).CreateQueryContext()
2024-11-26T07:47:34.3859271Z       /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/abci.go:1197 +0x1a9
2024-11-26T07:47:34.3860264Z   github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).getQueryCtx()
2024-11-26T07:47:34.3861705Z       /home/runner/work/minievm/minievm/jsonrpc/backend/tx.go:118 +0x192
2024-11-26T07:47:34.3862801Z   github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).feeFetcher.func1()
2024-11-26T07:47:34.3863766Z       /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:149 +0xba
2024-11-26T07:47:34.3865063Z   github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).feeFetcher()
2024-11-26T07:47:34.3866000Z       /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:179 +0x270
2024-11-26T07:47:34.3866976Z   github.com/initia-labs/minievm/jsonrpc/backend.NewJSONRPCBackend.gowrap1()
2024-11-26T07:47:34.3867835Z       /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:125 +0x33
2024-11-26T07:47:34.3868325Z 
2024-11-26T07:47:34.3868530Z Previous write at 0x00c091570220 by goroutine 3767:
2024-11-26T07:47:34.3869139Z   cosmossdk.io/store/rootmulti.(*Store).Commit()
2024-11-26T07:47:34.3869948Z       /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:482 +0x3b0
2024-11-26T07:47:34.3870721Z   github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Commit()
2024-11-26T07:47:34.3871790Z       /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/abci.go:933 +0x3da
2024-11-26T07:47:34.3872597Z   github.com/initia-labs/minievm/tests.ExecuteTxs()
2024-11-26T07:47:34.3873296Z       /home/runner/work/minievm/minievm/tests/tx_helper.go:206 +0x4f4
2024-11-26T07:47:34.3874110Z   github.com/initia-labs/minievm/jsonrpc/backend_test.Test_GetBalance()
2024-11-26T07:47:34.3874981Z       /home/runner/work/minievm/minievm/jsonrpc/backend/eth_test.go:27 +0x2af
2024-11-26T07:47:34.3875584Z   testing.tRunner()
2024-11-26T07:47:34.3876580Z       /home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1690 +0x226
2024-11-26T07:47:34.3877332Z   testing.(*T).Run.gowrap1()
2024-11-26T07:47:34.3878269Z       /home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1743 +0x44

case 2.

Read at 0x00c09d81cd40 by goroutine 4277:
  cosmossdk.io/store/rootmulti.(*Store).LastCommitID()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:449 +0x92
  cosmossdk.io/store/rootmulti.(*Store).LatestVersion()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:436 +0x26
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).CreateQueryContext()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/abci.go:1197 +0x1a9
  github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).getQueryCtx()
      /home/runner/work/minievm/minievm/jsonrpc/backend/tx.go:118 +0x192
  github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).feeFetcher.func1()
      /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:149 +0xba
  github.com/initia-labs/minievm/jsonrpc/backend.(*JSONRPCBackend).feeFetcher()
      /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:179 +0x270
  github.com/initia-labs/minievm/jsonrpc/backend.NewJSONRPCBackend.gowrap1()
      /home/runner/work/minievm/minievm/jsonrpc/backend/backend.go:125 +0x33

Previous write at 0x00c09d81cd40 by goroutine 3767:
  cosmossdk.io/store/rootmulti.commitStores()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:1216 +0x458
  cosmossdk.io/store/rootmulti.(*Store).Commit()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:482 +0x39b
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Commit()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/abci.go:933 +0x3da
  github.com/initia-labs/minievm/tests.ExecuteTxs()
      /home/runner/work/minievm/minievm/tests/tx_helper.go:206 +0x4f4
  github.com/initia-labs/minievm/jsonrpc/backend_test.Test_GetBalance()
      /home/runner/work/minievm/minievm/jsonrpc/backend/eth_test.go:27 +0x2af
  testing.tRunner()
      /home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      /home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1743 +0x44

see full logs

As I know, comet has entire lock between commit and query but GRPC still in this race condition risk I guess

cosmos-sdk/baseapp/grpcserver.go

Line 49 in 6dc6e8b

sdkCtx, err := app.CreateQueryContext(height, false)

Cosmos SDK Version

0.50

How to reproduce?

maybe hitting tons of grpc queries at block commit timing?

[beer-1]

another race condition

WARNING: DATA RACE
Write at 0x00c00021d560 by goroutine 23:
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).setState()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/baseapp.go:497 +0x51c
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).Commit()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/abci.go:956 +0x3e4
  github.com/cosmos/cosmos-sdk/baseapp_test.TestABCI_Race_Commit_Query()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/abci_test.go:2537 +0x490
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1690 +0x184
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:1743 +0x40

Previous read at 0x00c00021d560 by goroutine 72:
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).CreateQueryContext()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/abci.go:1233 +0x23c
  github.com/cosmos/cosmos-sdk/baseapp_test.TestABCI_Race_Commit_Query.func1()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/abci_test.go:2521 +0x7c

Goroutine 23 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:1743 +0x5e0
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:2168 +0x80
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1690 +0x184
  testing.runTests()
      /usr/local/go/src/testing/testing.go:2166 +0x6e0
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:2034 +0xb74
  main.main()
      _testmain.go:203 +0x110

Goroutine 72 (running) created at:
  github.com/cosmos/cosmos-sdk/baseapp_test.TestABCI_Race_Commit_Query()
      /Users/beer-1/Workspace/cosmos-sdk/baseapp/abci_test.go:2530 +0x404
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1690 +0x184
  testing.(*T).Run.gowrap1()
      /usr/local/go/src/testing/testing.go:1743 +0x40
==================

[beer-1]

test code I used (run this with race detector)

func TestABCI_Race_Commit_Query(t *testing.T) {
	suite := NewBaseAppSuite(t, baseapp.SetChainID("test-chain-id"))
	app := suite.baseApp

	_, err := app.InitChain(&abci.RequestInitChain{
		ChainId:         "test-chain-id",
		ConsensusParams: &cmtproto.ConsensusParams{Block: &cmtproto.BlockParams{MaxGas: 5000000}},
		InitialHeight:   1,
	})
	require.NoError(t, err)
	_, err = app.Commit()
	require.NoError(t, err)

	counter := atomic.Uint64{}
	counter.Store(0)

	ctx, cancel := context.WithCancel(context.Background())
	queryCreator := func() {
		for {
			select {
			case <-ctx.Done():
				return
			default:
				_, err := app.CreateQueryContext(0, false)
				require.NoError(t, err)

				counter.Add(1)
			}
		}
	}

	for i := 0; i < 100; i++ {
		go queryCreator()
	}

	for i := 0; i < 1000; i++ {
		_, err = app.FinalizeBlock(&abci.RequestFinalizeBlock{Height: app.LastBlockHeight() + 1})
		require.NoError(t, err)

		_, err = app.Commit()
		require.NoError(t, err)
	}

	cancel()

	require.Equal(t, int64(1001), app.GetContextForCheckTx(nil).BlockHeight())
}

[tac0turtle]

thank you for the issue, would you want to open a pull request on the sdk to add the test and a proposed fix?

[beer-1]

Okay, let me try it by EoW