alphagov/router — pull request #67 by alext (2013-12-09 10:44:55+00:00)
Title: Protect router mux pointer access with a mutex

Addressing the concerns raised in #66...

Pointer updates aren't guaranteed to be atomic by the [Go memory model](http://golang.org/ref/mem). It specifically says:

>Reads and writes of values larger than a single machine word behave as multiple machine-word-sized operations in an unspecified order.

Therefore pointer updates will only be atomic on architectures where the pointer size is the same as the machine word size. This is true for x86 and amd64, but may not be on other platforms (e.g. arm).

This PR protects accesses to the router's mux pointer with a mutex. This could have a small performance impact in a situation where the routes are being reloaded rapidly. Experimentation suggests that this is negligible. I did 3 runs of the "when the routes are being reloaded repeatedly" performance test with the [sleep](https://github.com/alphagov/router/blob/master/spec/performance_spec.rb#L58) removed with and without the mutex in place:

Without mutex:
```
when the routes are being reloaded repeatedly latencies
{:direct=>{"mean"=> 478097, "95th"=> 742817, "99th"=>1946298, "max"=>1946298},
:router=>{"mean"=>1074822, "95th"=>1692920, "99th"=>4894379, "max"=>4894379}}
{:direct=>{"mean"=> 453925, "95th"=> 722982, "99th"=>1927301, "max"=>1927301},
:router=>{"mean"=>1067174, "95th"=>1855786, "99th"=>5354484, "max"=>5354484}}
{:direct=>{"mean"=> 492856, "95th"=> 794778, "99th"=>2514795, "max"=>2514795},
:router=>{"mean"=>1166290, "95th"=>1848523, "99th"=>3316277, "max"=>3316277}}
```

With the mutex:
```
when the routes are being reloaded repeatedly latencies
{:direct=>{"mean"=> 444932, "95th"=> 659847, "99th"=>2236273, "max"=>2236273},
:router=>{"mean"=>1118250, "95th"=>1879252, "99th"=>6842355, "max"=>6842355}}
{:direct=>{"mean"=> 436670, "95th"=> 744214, "99th"=>2130117, "max"=>2130117},
:router=>{"mean"=>1143926, "95th"=>1686214, "99th"=>8259546, "max"=>8259546}}
{:direct=>{"mean"=> 405500, "95th"=> 788352, "99th"=>2313097, "max"=>2313097},
:router=>{"mean"=>1114122, "95th"=>1869753, "99th"=>8516276, "max"=>8516276}}
```

This shows a vary small increase in the added latency for this pathological case. I don't think this will negatively impact performance in any real way.