Commit3a48e40

authored

fix: Race when shutting down and opening WebSockets (#576)

Adding to a WaitGroup while calling wait is a race condition. Surroundingthis in a mutex should solve the problem. Since context is used forcancellation on all sockets, cleanup should occur properly.See:https://github.com/coder/coder/runs/5701221057?check_suite_focus=true#step:10:98

1 parent4448ba2 commit3a48e40Copy full SHA for 3a48e40

File tree

+12

-1

lines changed

+12

-1

lines changed

Lines changed: 6 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -176,13 +176,18 @@ func New(options *Options) (http.Handler, func()) {`
`176`	`176`	`})`
`177`	`177`	`})`
`178`	`178`	`r.NotFound(site.DefaultHandler().ServeHTTP)`
`179`		`-returnr,api.websocketWaitGroup.Wait`
	`179`	`+returnr,func() {`
	`180`	`+api.websocketWaitMutex.Lock()`
	`181`	`+api.websocketWaitGroup.Wait()`
	`182`	`+api.websocketWaitMutex.Unlock()`
	`183`	`+}`
`180`	`184`	`}`
`181`	`185`
`182`	`186`	`// API contains all route handlers. Only HTTP handlers should`
`183`	`187`	`// be added to this struct for code clarity.`
`184`	`188`	`typeapistruct {`
`185`	`189`	`*Options`
`186`	`190`
	`191`	`+websocketWaitMutex sync.Mutex`
`187`	`192`	`websocketWaitGroup sync.WaitGroup`
`188`	`193`	`}`

Lines changed: 2 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,9 @@ import (`
`33`	`33`
`34`	`34`	`// Serves the provisioner daemon protobuf API over a WebSocket.`
`35`	`35`	`func (apiapi)provisionerDaemonsListen(rw http.ResponseWriter,rhttp.Request) {`
	`36`	`+api.websocketWaitMutex.Lock()`
`36`	`37`	`api.websocketWaitGroup.Add(1)`
	`38`	`+api.websocketWaitMutex.Unlock()`
`37`	`39`	`deferapi.websocketWaitGroup.Done()`
`38`	`40`
`39`	`41`	`conn,err:=websocket.Accept(rw,r,&websocket.AcceptOptions{`

Lines changed: 4 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,9 @@ func (api api) workspaceResource(rw http.ResponseWriter, r http.Request) {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`func (apiapi)workspaceResourceDial(rw http.ResponseWriter,rhttp.Request) {`
	`67`	`+api.websocketWaitMutex.Lock()`
`67`	`68`	`api.websocketWaitGroup.Add(1)`
	`69`	`+api.websocketWaitMutex.Unlock()`
`68`	`70`	`deferapi.websocketWaitGroup.Done()`
`69`	`71`
`70`	`72`	`resource:=httpmw.WorkspaceResourceParam(r)`
`@@ -112,7 +114,9 @@ func (api api) workspaceResourceDial(rw http.ResponseWriter, r http.Request) {`
`112`	`114`	`}`
`113`	`115`
`114`	`116`	`func (apiapi)workspaceAgentListen(rw http.ResponseWriter,rhttp.Request) {`
	`117`	`+api.websocketWaitMutex.Lock()`
`115`	`118`	`api.websocketWaitGroup.Add(1)`
	`119`	`+api.websocketWaitMutex.Unlock()`
`116`	`120`	`deferapi.websocketWaitGroup.Done()`
`117`	`121`
`118`	`122`	`agent:=httpmw.WorkspaceAgent(r)`

Comments

(0)