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

[BUG] Unprotected read data race #1040

Open
corverroos opened this issue Jan 23, 2025 · 2 comments
Open

[BUG] Unprotected read data race #1040

corverroos opened this issue Jan 23, 2025 · 2 comments

Comments

@corverroos
Copy link
Contributor

corverroos commented Jan 23, 2025 •
edited
Loading

We switched from using the cometBFT ABCI RPC to using CosmosSDK gRPC to query application state in this omni-network/omni#2216.

Our CI then started failing with the following data race at: iavl@v1.9.11/nodedb.go:751

See stack trace below.

I opened this PR to fix it, but the PR got stuck in review, with additional data races being highlighted by Code Rabbit.

I closed that PR with the hope that the cosmos team will handle this issue. 🙏
@corverroos
Copy link
Contributor Author 

==================
WARNING: DATA RACE
Write at 0x00c002c7a758 by goroutine 201:
  github.com/cosmos/iavl.(*nodeDB).resetLatestVersion()
      /home/runner/go/pkg/mod/github.com/omni-network/iavl@v1.9.11/nodedb.go:751 +0x99
  github.com/cosmos/iavl.(*MutableTree).SaveVersion()
      /home/runner/go/pkg/mod/github.com/omni-network/iavl@v1.9.11/mutable_tree.go:763 +0x108b
  cosmossdk.io/store/iavl.(*Store).Commit()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/iavl/store.go:126 +0x10e
  cosmossdk.io/store/cache.(*CommitKVStoreCache).Commit()
      <autogenerated>:1 +0x43
  cosmossdk.io/store/rootmulti.commitStores()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/rootmulti/store.go:1196 +0x196
  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/omni-network/omni/halo/app.(*App).Commit()
      <autogenerated>:1 +0x52
  github.com/cosmos/cosmos-sdk/server.cometABCIWrapper.Commit()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/server/cmt_abci.go:56 +0x4a
  github.com/cosmos/cosmos-sdk/server.(*cometABCIWrapper).Commit()
      <autogenerated>:1 +0x1b
  github.com/omni-network/omni/halo/app.abciWrapper.Commit()
      /home/runner/work/omni/omni/halo/app/abci.go:160 +0xa5
  github.com/omni-network/omni/halo/app.(*abciWrapper).Commit()
      <autogenerated>:1 +0x7b
  github.com/cometbft/cometbft/abci/client.(*localClient).Commit()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/abci/client/local_client.go:113 +0x158
  github.com/cometbft/cometbft/proxy.(*appConnConsensus).Commit()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/proxy/app_conn.go:109 +0x2cb
  github.com/cometbft/cometbft/state.(*BlockExecutor).Commit()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/state/execution.go:404 +0x368
  github.com/cometbft/cometbft/state.(*BlockExecutor).applyBlock()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/state/execution.go:290 +0x1a39
  github.com/cometbft/cometbft/state.(*BlockExecutor).ApplyVerifiedBlock()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/state/execution.go:202 +0x143e
  github.com/cometbft/cometbft/consensus.(*State).finalizeCommit()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:1772 +0x1214
  github.com/cometbft/cometbft/consensus.(*State).tryFinalizeCommit()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:1682 +0x4a4
  github.com/cometbft/cometbft/consensus.(*State).enterCommit.func1()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:1617 +0x114
  runtime.deferreturn()
      /opt/hostedtoolcache/go/1.23.2/x64/src/runtime/panic.go:605 +0x5d
  github.com/cometbft/cometbft/consensus.(*State).addVote()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:2335 +0x36c4
  github.com/cometbft/cometbft/consensus.(*State).tryAddVote()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:2067 +0x6b
  github.com/cometbft/cometbft/consensus.(*State).handleMsg()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:929 +0x60f
  github.com/cometbft/cometbft/consensus.(*State).receiveRoutine()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:856 +0x751
  github.com/cometbft/cometbft/consensus.(*State).OnStart.gowrap1()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:398 +0x35

Previous read at 0x00c002c7a758 by goroutine 848:
  github.com/cosmos/iavl.(*ImmutableTree).Get()
      /home/runner/go/pkg/mod/github.com/omni-network/iavl@v1.9.11/immutable_tree.go:198 +0x287
  cosmossdk.io/store/iavl.(*immutableTree).Get()
      <autogenerated>:1 +0x64
  cosmossdk.io/store/iavl.(*Store).Get()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/iavl/store.go:200 +0x144
  cosmossdk.io/store/cachekv.(*Store).Get()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/cachekv/store.go:61 +0x26a
  cosmossdk.io/store/gaskv.(*Store).Get()
      /home/runner/go/pkg/mod/cosmossdk.io/store@v1.1.1/gaskv/store.go:37 +0xad
  github.com/cosmos/cosmos-sdk/runtime.coreKVStore.Get()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/runtime/store.go:65 +0x6f
  github.com/cosmos/cosmos-sdk/runtime.(*coreKVStore).Get()
      <autogenerated>:1 +0x1f
  cosmossdk.io/orm/model/ormtable.primaryKeyIndex.getByKeyBytes()
      /home/runner/go/pkg/mod/cosmossdk.io/orm@v1.0.0-beta.3/model/ormtable/primary_key.go:182 +0x95
  cosmossdk.io/orm/model/ormtable.primaryKeyIndex.get()
      /home/runner/go/pkg/mod/cosmossdk.io/orm@v1.0.0-beta.3/model/ormtable/primary_key.go:77 +0x1c7
  cosmossdk.io/orm/model/ormtable.primaryKeyIndex.Get()
      /home/runner/go/pkg/mod/cosmossdk.io/orm@v1.0.0-beta.3/model/ormtable/primary_key.go:68 +0x1b1
  cosmossdk.io/orm/model/ormtable.(*primaryKeyIndex).Get()
      <autogenerated>:1 +0x156
  github.com/omni-network/omni/halo/portal/keeper.blockTable.Get()
      /home/runner/work/omni/omni/halo/portal/keeper/portal.cosmos_orm.go:112 +0x161
  github.com/omni-network/omni/halo/portal/keeper.(*blockTable).Get()
      <autogenerated>:1 +0x6b
  github.com/omni-network/omni/halo/portal/keeper.Keeper.getBlockAndMsgs()
      /home/runner/work/omni/omni/halo/portal/keeper/keeper.go:109 +0xa1
  github.com/omni-network/omni/halo/portal/keeper.Keeper.Block()
      /home/runner/work/omni/omni/halo/portal/keeper/query.go:26 +0x166
  github.com/omni-network/omni/halo/portal/keeper.(*Keeper).Block()
      <autogenerated>:1 +0xcf
  github.com/omni-network/omni/halo/portal/types._Query_Block_Handler.func1()
      /home/runner/work/omni/omni/halo/portal/types/query.pb.go:325 +0xbe
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).RegisterGRPCServer.func1()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/grpcserver.go:67 +0x4eb
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).RegisterGRPCServer.func2.ChainUnaryServer.2.1()
      /home/runner/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/chain.go:48 +0xd0
  github.com/grpc-ecosystem/go-grpc-middleware/recovery.UnaryServerInterceptor.func1()
      /home/runner/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/recovery/interceptors.go:33 +0x174
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).RegisterGRPCServer.func2.ChainUnaryServer.2()
      /home/runner/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware@v1.4.0/chain.go:53 +0x22c
  github.com/omni-network/omni/halo/portal/types._Query_Block_Handler()
      /home/runner/work/omni/omni/halo/portal/types/query.pb.go:327 +0x1f3
  github.com/cosmos/cosmos-sdk/baseapp.(*BaseApp).RegisterGRPCServer.func2()
      /home/runner/go/pkg/mod/github.com/cosmos/cosmos-sdk@v0.50.10/baseapp/grpcserver.go:81 +0x1b5
  google.golang.org/grpc.(*Server).processUnaryRPC()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:1[394](https://github.com/omni-network/omni/actions/runs/11408111384/job/31745600674#step:6:395) +0x1b4b
  google.golang.org/grpc.(*Server).handleStream()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:1805 +0x1824
  google.golang.org/grpc.(*Server).serveStreams.func2.1()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:1029 +0x158

Goroutine 201 (running) created at:
  github.com/cometbft/cometbft/consensus.(*State).OnStart()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/state.go:[398](https://github.com/omni-network/omni/actions/runs/11408111384/job/31745600674#step:6:399) +0x22a
  github.com/cometbft/cometbft/libs/service.(*BaseService).Start()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/libs/service/service.go:144 +0x583
  github.com/cometbft/cometbft/consensus.(*Reactor).OnStart()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/consensus/reactor.go:84 +0x214
  github.com/cometbft/cometbft/libs/service.(*BaseService).Start()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/libs/service/service.go:144 +0x583
  github.com/cometbft/cometbft/consensus.(*Reactor).Start()
      <autogenerated>:1 +0x31
  github.com/cometbft/cometbft/p2p.(*Switch).OnStart()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/p2p/switch.go:237 +0x115
  github.com/cometbft/cometbft/libs/service.(*BaseService).Start()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/libs/service/service.go:144 +0x583
  github.com/cometbft/cometbft/node.(*Node).OnStart()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/node/node.go:558 +0x70b
  github.com/cometbft/cometbft/libs/service.(*BaseService).Start()
      /home/runner/go/pkg/mod/github.com/cometbft/cometbft@v0.38.12/libs/service/service.go:144 +0x583
  github.com/omni-network/omni/halo/app.Start()
      /home/runner/work/omni/omni/halo/app/start.go:207 +0x1784
  github.com/omni-network/omni/halo/app_test.TestPruningHistory()
      /home/runner/work/omni/omni/halo/app/pruning_test.go:38 +0x207
  testing.tRunner()
      /opt/hostedtoolcache/go/1.23.2/x64/src/testing/testing.go:1690 +0x226
  testing.(*T).Run.gowrap1()
      /opt/hostedtoolcache/go/1.23.2/x64/src/testing/testing.go:1743 +0x44

Goroutine 848 (finished) created at:
  google.golang.org/grpc.(*Server).serveStreams.func2()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:1040 +0x224
  google.golang.org/grpc/internal/transport.(*http2Server).operateHeaders()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/internal/transport/http2_server.go:630 +0x3881
  google.golang.org/grpc/internal/transport.(*http2Server).HandleStreams()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/internal/transport/http2_server.go:671 +0x[415](https://github.com/omni-network/omni/actions/runs/11408111384/job/31745600674#step:6:416)
  google.golang.org/grpc.(*Server).serveStreams()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:1023 +0x69b
  google.golang.org/grpc.(*Server).handleRawConn.func1()
      /home/runner/go/pkg/mod/google.golang.org/grpc@v1.67.1/server.go:958 +0x86
==================

@corverroos corverroos mentioned this issue Jan 23, 2025
Closed
@corverroos
Copy link
Contributor Author

Note that there are additional data races in CosmosSDK itself when querying gRPC directly (instead of via CometBFT ABCI which has a lock).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@corverroos