From 8344b8a29ecef44663897b78768fd99f58def512 Mon Sep 17 00:00:00 2001 From: Carl Jackson Date: Mon, 28 Jul 2014 02:00:58 -0700 Subject: [PATCH] Fix race in state machine compilation/invalidation Previously, a state machine invalidation could have raced against an in-flight routing attempt: if the invalidation occured after the routing attempt had already completed its nil-check (choosing not to compile a new state machine) but before the state machine was atomically loaded to perform routing, the routing goroutine would begin to panic from dereferencing nil. The meat of this change is that we now return the state machine that we compiled (while still holding the lock), and we only ever interact with the state machine through atomic pointer loads. --- web/atomic.go | 4 ++-- web/router.go | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/web/atomic.go b/web/atomic.go index 1bbf48e..aab483a 100644 --- a/web/atomic.go +++ b/web/atomic.go @@ -5,10 +5,10 @@ import ( "unsafe" ) -func (rt *router) getMachine() routeMachine { +func (rt *router) getMachine() *routeMachine { ptr := (*unsafe.Pointer)(unsafe.Pointer(&rt.machine)) sm := (*routeMachine)(atomic.LoadPointer(ptr)) - return *sm + return sm } func (rt *router) setMachine(m *routeMachine) { ptr := (*unsafe.Pointer)(unsafe.Pointer(&rt.machine)) diff --git a/web/router.go b/web/router.go index 137301c..cc52d96 100644 --- a/web/router.go +++ b/web/router.go @@ -231,7 +231,7 @@ func (rm routeMachine) route(c *C, w http.ResponseWriter, r *http.Request) (meth // after all the routes have been added, and will be called automatically for // you (at some performance cost on the first request) if you do not call it // explicitly. -func (rt *router) Compile() { +func (rt *router) Compile() *routeMachine { rt.lock.Lock() defer rt.lock.Unlock() sm := routeMachine{ @@ -239,14 +239,16 @@ func (rt *router) Compile() { routes: rt.routes, } rt.setMachine(&sm) + return &sm } func (rt *router) route(c *C, w http.ResponseWriter, r *http.Request) { - if rt.machine == nil { - rt.Compile() + rm := rt.getMachine() + if rm == nil { + rm = rt.Compile() } - methods, ok := rt.getMachine().route(c, w, r) + methods, ok := rm.route(c, w, r) if ok { return }