Skip to content

fix: Take a read lock rather than an exclusive lock in getContextForTx - #26800

Open
SillyZir wants to merge 1 commit into
cosmos:mainfrom
SillyZir:auto-fix/24634
Open

fix: Take a read lock rather than an exclusive lock in getContextForTx#26800
SillyZir wants to merge 1 commit into
cosmos:mainfrom
SillyZir:auto-fix/24634

Conversation

@SillyZir

@SillyZir SillyZir commented Sep 3, 2026

Copy link
Copy Markdown

Description

Ref: #24634 (partial — see Scope)

getContextForTx acquired app.mu exclusively, but reads only:

  • app.stateManager.GetState(mode) — the state manager is independently
    synchronised by its own sync.RWMutex (baseapp/state/manager.go)
  • app.sigverifyTx
  • app.GetConsensusParams(ctx), which does not touch app.mu

app.mu documents itself as protecting name, version and appVersion.
Every other use in baseapp respects that scope — the readers (Name,
Version, AppVersion, Info) take RLock, and the only writers are the
setup-time setters in options.go. getContextForTx was the sole exception.

Since it is called per transaction from runTx, that exclusive acquisition
serialised transaction execution against those readers for no reason.

Scope

This does not address the stateManager.stateMut contention #24634 also
implies. CreateQueryContextWithCheckHeader does not take app.mu on its hot
path, so query latency specifically is unaffected except via Name() on the
"not ready" branch. Filed as a partial improvement rather than a fix — happy to
close it if you would rather see the issue addressed as a whole.

Testing

go test -race ./baseapp/... passes (baseapp 185s, plus blockexec, oe,
txnrunner). No regression test: the change narrows a lock scope without
altering behaviour, so no input fails before and passes after.

getContextForTx acquired app.mu exclusively while reading only the state
manager (which is independently synchronised by its own RWMutex) and
app.sigverifyTx. app.mu documents itself as protecting name, version and
appVersion; every other site in baseapp respects that scope, and this was
the only writer outside the setup-time setters in options.go.

Downgrading to a read lock removes a per-transaction exclusive acquisition
from the execution path, so it no longer serialises against Name, Version,
AppVersion and Info.

Refs cosmos#24634
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

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

Successfully merging this pull request may close these issues.

1 participant