Skip to content

Commit

Permalink
Fix data race in root endpoint (#2745)
Browse files Browse the repository at this point in the history
There is a data race in the root endpoint when the action accesses action.App.coreVersion and other properties from action.App. These properties are updated in the App.Tick() goroutine. This commit fixes the data race by wrapping these properties in a read write lock. Whenever the properties are updated the write lock is acquired. Similarly, whenever the properties are read the read lock is acquired.
  • Loading branch information
tamirms authored Jun 25, 2020
1 parent 3a30dc0 commit 8a6476e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 22 deletions.
7 changes: 4 additions & 3 deletions services/horizon/internal/actions_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,16 @@ func (action *RootAction) JSON() error {
"strictReceivePaths": StrictReceivePathsQuery{}.URITemplate(),
"strictSendPaths": FindFixedPathsQuery{}.URITemplate(),
}
coreInfo := action.App.coreSettings.get()
resourceadapter.PopulateRoot(
action.R.Context(),
&res,
ledger.CurrentState(),
action.App.horizonVersion,
action.App.coreVersion,
coreInfo.coreVersion,
action.App.config.NetworkPassphrase,
action.App.currentProtocolVersion,
action.App.coreSupportedProtocolVersion,
coreInfo.currentProtocolVersion,
coreInfo.coreSupportedProtocolVersion,
action.App.config.FriendbotURL,
templates,
)
Expand Down
60 changes: 41 additions & 19 deletions services/horizon/internal/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

metrics "github.com/rcrowley/go-metrics"
"github.com/stellar/go/clients/stellarcore"
proto "github.com/stellar/go/protocols/stellarcore"
horizonContext "github.com/stellar/go/services/horizon/internal/context"
"github.com/stellar/go/services/horizon/internal/db2/core"
"github.com/stellar/go/services/horizon/internal/db2/history"
Expand All @@ -30,24 +31,47 @@ import (
graceful "gopkg.in/tylerb/graceful.v1"
)

// App represents the root of the state of a horizon instance.
type App struct {
config Config
web *web
historyQ *history.Q
coreQ *core.Q
ctx context.Context
cancel func()
coreVersion string
horizonVersion string
type coreSettings struct {
currentProtocolVersion int32
coreSupportedProtocolVersion int32
orderBookStream *expingest.OrderBookStream
submitter *txsub.System
paths paths.Finder
expingester *expingest.System
reaper *reap.System
ticks *time.Ticker
coreVersion string
}

type coreSettingsStore struct {
sync.RWMutex
coreSettings
}

func (c *coreSettingsStore) set(resp *proto.InfoResponse) {
c.Lock()
defer c.Unlock()
c.coreVersion = resp.Info.Build
c.currentProtocolVersion = int32(resp.Info.Ledger.Version)
c.coreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion)
}

func (c *coreSettingsStore) get() coreSettings {
c.RLock()
defer c.RUnlock()
return c.coreSettings
}

// App represents the root of the state of a horizon instance.
type App struct {
config Config
web *web
historyQ *history.Q
coreQ *core.Q
ctx context.Context
cancel func()
horizonVersion string
coreSettings coreSettingsStore
orderBookStream *expingest.OrderBookStream
submitter *txsub.System
paths paths.Finder
expingester *expingest.System
reaper *reap.System
ticks *time.Ticker

// metrics
metrics metrics.Registry
Expand Down Expand Up @@ -384,9 +408,7 @@ func (a *App) UpdateStellarCoreInfo() {
os.Exit(1)
}

a.coreVersion = resp.Info.Build
a.currentProtocolVersion = int32(resp.Info.Ledger.Version)
a.coreSupportedProtocolVersion = int32(resp.Info.ProtocolVersion)
a.coreSettings.set(resp)
}

// UpdateMetrics triggers a refresh of several metrics gauges, such as open
Expand Down

0 comments on commit 8a6476e

Please sign in to comment.