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.

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

Fix MuxHandler initialization race #1627

Merged
merged 1 commit into from Mar 23, 2018

Conversation

Projects
None yet
3 participants
@rwool
Copy link
Contributor

rwool commented Mar 23, 2018

Fix race condition that can occur when calling handlers returned from
Controller.MuxHandler where the Service's slice of middleware functions
is written to in a way that is not thread-safe.

The race will only occur when the capacity of the Service's middleware
slice is greater than its length and when the number of Controller
middlewares is more than 0. The functions returned by MuxHandler (that
are all from the same Controller object), if called in parallel, could
cause the race to occur.

(cherry picked from commit cfffe9c)

Fix MuxHandler initialization race (#1626)
Fix race condition that can occur when calling handlers returned from
Controller.MuxHandler where the Service's slice of middleware functions
is written to in a way that is not thread-safe.

The race will only occur when the capacity of the Service's middleware
slice is greater than its length and when the number of Controller
middlewares is more than 0. The functions returned by MuxHandler (that
are all from the same Controller object), if called in parallel, could
cause the race to occur.

(cherry picked from commit cfffe9c)
@raphael

This comment has been minimized.

Copy link
Member

raphael commented Mar 23, 2018

Thank you!

@raphael raphael merged commit 854ef89 into goadesign:v1 Mar 23, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rwool rwool deleted the rwool:fix-muxhandler-init-race-v1 branch Mar 23, 2018

@jiekang jiekang referenced this pull request Jul 13, 2018

Closed

Add 1.x.y release tag #1807

@dbsheehy

This comment has been minimized.

Copy link

dbsheehy commented Jan 28, 2019

Just want to give you guys a heads up that I don't think this bug has been completely fixed. I'm seeing this bug on a build using post 1.4.0 goa code:

INFO[01-23|12:10:29] serve file m=rest req_id=XQRp7nWCtN-7 name=gui.tmp/axios.min.js route=/apigui/axios.min.js
INFO[01-23|12:10:29] completed m=rest req_id=XQRp7nWCtN-7 status=304 bytes=0 time=1.936894ms ctrl=GuiController action=serve

WARNING: DATA RACE
Read at 0x00c4202061d0 by goroutine 21:
apeiron/asm/vendor/github.com/goadesign/goa.(*Controller).MuxHandler.func1()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/service.go:251 +0x9d
apeiron/asm/vendor/github.com/goadesign/goa.(*mux).Handle.func1()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/mux.go:59 +0x23e
apeiron/asm/vendor/github.com/dimfeld/httptreemux.(*TreeMux).ServeLookupResult()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/dimfeld/httptreemux/router.go:247 +0x195
apeiron/asm/vendor/github.com/dimfeld/httptreemux.(*TreeMux).ServeHTTP()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/dimfeld/httptreemux/router.go:268 +0x155
apeiron/asm/vendor/github.com/goadesign/goa.(*mux).ServeHTTP()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/mux.go:85 +0x67
net/http.serverHandler.ServeHTTP()
/home/david/go/go1.10.2/src/net/http/server.go:2694 +0xb9
net/http.(*conn).serve()
/home/david/go/go1.10.2/src/net/http/server.go:1830 +0x7dc

Previous write at 0x00c4202061d0 by goroutine 22:
apeiron/asm/vendor/github.com/goadesign/goa.(*Controller).MuxHandler.func1()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/service.go:252 +0x131
apeiron/asm/vendor/github.com/goadesign/goa.(*mux).Handle.func1()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/mux.go:59 +0x23e
apeiron/asm/vendor/github.com/dimfeld/httptreemux.(*TreeMux).ServeLookupResult()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/dimfeld/httptreemux/router.go:247 +0x195
apeiron/asm/vendor/github.com/dimfeld/httptreemux.(*TreeMux).ServeHTTP()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/dimfeld/httptreemux/router.go:268 +0x155
apeiron/asm/vendor/github.com/goadesign/goa.(*mux).ServeHTTP()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/mux.go:85 +0x67
net/http.serverHandler.ServeHTTP()
/home/david/go/go1.10.2/src/net/http/server.go:2694 +0xb9
net/http.(*conn).serve()
/home/david/go/go1.10.2/src/net/http/server.go:1830 +0x7dc

Goroutine 21 (running) created at:
net/http.(*Server).Serve()
/home/david/go/go1.10.2/src/net/http/server.go:2795 +0x364
net/http.(*Server).ListenAndServe()
/home/david/go/go1.10.2/src/net/http/server.go:2711 +0xc4
net/http.ListenAndServe()
/home/david/go/go1.10.2/src/net/http/server.go:2969 +0xf6
apeiron/asm/vendor/github.com/goadesign/goa.(*Service).ListenAndServe()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/service.go:151 +0x206
apeiron/asm/server/restapi.StartServer()
/home/david/prj/go/src/apeiron/asm/server/restapi/restapi.go:57 +0x727

Goroutine 22 (running) created at:
net/http.(*Server).Serve()
/home/david/go/go1.10.2/src/net/http/server.go:2795 +0x364
net/http.(*Server).ListenAndServe()
/home/david/go/go1.10.2/src/net/http/server.go:2711 +0xc4
net/http.ListenAndServe()
/home/david/go/go1.10.2/src/net/http/server.go:2969 +0xf6
apeiron/asm/vendor/github.com/goadesign/goa.(*Service).ListenAndServe()
/home/david/prj/go/src/apeiron/asm/vendor/github.com/goadesign/goa/service.go:151 +0x206
apeiron/asm/server/restapi.StartServer()
/home/david/prj/go/src/apeiron/asm/server/restapi/restapi.go:57 +0x727

I don't have anything I can give you guys that can be used to reproduce this issue right now. But when I get a chance I'll see if I can get something to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment