Prevent concurrent read and writes to the API config

Review Request #9834 — Created March 26, 2018 and submitted

Barret Rennie
rb-gateway
master
9795
9839
3ff0120...
rb-gateway

Previously it was possible for the API's configuration to be modified
while it was serving a request, resulting in data races and undefined
behaviour if we read the repository map part-way through a modification.

Now, we wrap all modification to the configuration with a RWMutex. It
normal operation, we only acquire the mutex for reading once -- done by
api.Serve() (which is now the replacement for creating an
http.Server and managing it), which will be released when we finish
serving. Setting the configuration now requires write-access to the lock
(so that we cannot concurrently be serving a request and update the
configuration).

Since we want to maintain these guarantees in unit tests without running
an actual HTTP server, api.ServeHTTP() now acquires the read portion
of the lock before serving a request. However, this function is now only
used in unit tests -- api.Serve() uses the router as the handler
instead of itself so that it doesn't constantly acquire and release
locks while serving requests.

Ran unit tests.
Made HTTP requests against rb-gateway and saw correct responses.

  • 0
  • 0
  • 1
  • 1
  • 2
Description From Last Updated
Barret Rennie
David Trowbridge
  1. Ship It!

  2. 
      
Barret Rennie
Barret Rennie
Barret Rennie
Barret Rennie
David Trowbridge
  1. 
      
  2. api/api.go (Diff revision 5)
     
     

    Shouldn't this include err?

  3. 
      
Barret Rennie
David Trowbridge
  1. 
      
  2. api/api.go (Diff revision 6)
     
     

    Since this is only used in tests, can we make it un-exported?

    1. We can't, the unit tests are in the api_tests package (to avoid circular dependencies on helpers -> api -> (unit tests) -> helpers .... )

  3. 
      
David Trowbridge
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (923a1e4)
Loading...