NotificationsYou must be signed in to change notification settings
Fork907
Star10k

Commitef00ae5

authored

fix: fix data race in agentscripts.Runner (#17630)

Fixescoder/internal#604Fixes a data race in `agentscripts.Runner` where a concurrent `Execute()` call races with `Init()`. We hit this race during shut down, which is not synchronized against starting up.In this PR I've chosen to add synchronization to the `Runner` rather than try to synchronize the calls in the agent. When we close down the agent, it's OK to just throw an error if we were never initialized with a startup script---we don't want to wait for it since that requires an active connection to the control plane.

1 parent35d686c commitef00ae5Copy full SHA for ef00ae5

File tree

1 file changed

+19

-4

lines changed

agent/agentscripts
- agentscripts.go

1 file changed

+19

-4

lines changed

`‎agent/agentscripts/agentscripts.go`

Lines changed: 19 additions & 4 deletions

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,6 @@ import (`
`10`	`10`	`"os/user"`
`11`	`11`	`"path/filepath"`
`12`	`12`	`"sync"`
`13`		`-"sync/atomic"`
`14`	`13`	`"time"`
`15`	`14`
`16`	`15`	`"github.com/google/uuid"`
`@@ -104,7 +103,6 @@ type Runner struct {`
`104`	`103`	`closedchanstruct{}`
`105`	`104`	`closeMutex sync.Mutex`
`106`	`105`	`cron*cron.Cron`
`107`		`-initialized atomic.Bool`
`108`	`106`	`scripts []runnerScript`
`109`	`107`	`dataDirstring`
`110`	`108`	`scriptCompletedScriptCompletedFunc`
`@@ -113,6 +111,9 @@ type Runner struct {`
`113`	`111`	`// execute startup scripts, and scripts on a cron schedule. Both will increment`
`114`	`112`	`// this counter.`
`115`	`113`	`scriptsExecuted*prometheus.CounterVec`
	`114`	`+`
	`115`	`+initMutex sync.Mutex`
	`116`	`+initializedbool`
`116`	`117`	`}`
`117`	`118`
`118`	`119`	`// DataDir returns the directory where scripts data is stored.`
`@@ -154,10 +155,12 @@ func WithPostStartScripts(scripts ...codersdk.WorkspaceAgentScript) InitOption {`
`154`	`155`	`// It also schedules any scripts that have a schedule.`
`155`	`156`	`// This function must be called before Execute.`
`156`	`157`	`func (r*Runner)Init(scripts []codersdk.WorkspaceAgentScript,scriptCompletedScriptCompletedFunc,opts...InitOption)error {`
`157`		`-ifr.initialized.Load() {`
	`158`	`+r.initMutex.Lock()`
	`159`	`+deferr.initMutex.Unlock()`
	`160`	`+ifr.initialized {`
`158`	`161`	`returnxerrors.New("init: already initialized")`
`159`	`162`	`}`
`160`		`-r.initialized.Store(true)`
	`163`	`+r.initialized=true`
`161`	`164`	`r.scripts=toRunnerScript(scripts...)`
`162`	`165`	`r.scriptCompleted=scriptCompleted`
`163`	`166`	`for_,opt:=rangeopts {`
`@@ -227,6 +230,18 @@ const (`
`227`	`230`
`228`	`231`	`// Execute runs a set of scripts according to a filter.`
`229`	`232`	`func (r*Runner)Execute(ctx context.Context,optionExecuteOption)error {`
	`233`	`+initErr:=func()error {`
	`234`	`+r.initMutex.Lock()`
	`235`	`+deferr.initMutex.Unlock()`
	`236`	`+if!r.initialized {`
	`237`	`+returnxerrors.New("execute: not initialized")`
	`238`	`+}`
	`239`	`+returnnil`
	`240`	`+}()`
	`241`	`+ifinitErr!=nil {`
	`242`	`+returninitErr`
	`243`	`+}`
	`244`	`+`
`230`	`245`	`vareg errgroup.Group`
`231`	`246`	`for_,script:=ranger.scripts {`
`232`	`247`	`runScript:= (option==ExecuteStartScripts&&script.RunOnStart)\|\|`

0 commit comments

Comments

(0)

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commitef00ae5

File tree

1 file changed

1 file changed

`‎agent/agentscripts/agentscripts.go`

0 commit comments