Singleton global object cannot be changed by multiple threads without a mutex or data access conflicts will occur

bug-singleton-global-data-access.md

I was researching how to use Golang's Template.FuncMap() and found a data access issue in: https://www.calhoun.io/intro-to-templates-p3-functions/

The section in question:

# Making our functions globally useful

Next we need to define our function that uses a closure. This is basically a fancy way of saying we are
going to define a dynamic function that has access to variables that are not necessarily passed into it,
but are available when we define the function. 

A Go Template object is created as global singleton:

var testTemplate *template.Template

A temporary hasPermission() function is defined during initialization so the hello.gohtml template can be correctly parsed.

	testTemplate, err = template.New("hello.gohtml").Funcs(template.FuncMap{
		"hasPermission": func(feature string) bool {
			return false
		},

The intention is to change out the hasPermission function for each request. Changing the FuncMap is specifically allowed in the Template.Funcs() documentation: "It is legal to overwrite elements of the map"

This closure captures a User object and this function is passed the global singleton testTemplate. This works correctly during tests. The User object contains the expected "correct" (AKA fake) user data.

Not under load though. Do not assume strict goroutine procedural order in a multiprocessing environment. Template.Funcs() can be called by two goroutines in nearly the same moment, so that later when they separately call Template.Execute() they will call the exact same hasPermission() function. hasPermission() function can be changed "out from under" the request. This can result in any number of security bugs or other serious issues in a production application.

See code/comments below for details of a modified example that will Fatal() during data access problems.

hello.gohtml

{{ .User.ID }}
{{if hasPermission "feature-a"}}
  <div class="feature">
    <h3>Feature A</h3>
    <p>Some other stuff here...</p>
  </div>
{{else}}
  <div class="feature disabled">
    <h3>Feature A</h3>
    <p>To enable Feature A please upgrade your plan</p>
  </div>
{{end}}

{{if hasPermission "feature-b"}}
  <div class="feature">
    <h3>Feature B</h3>
    <p>Some other stuff here...</p>
  </div>
{{else}}
  <div class="feature disabled">
    <h3>Feature B</h3>
    <p>To enable Feature B please upgrade your plan</p>
  </div>
{{end}}

main.go

package main
// src: https://www.calhoun.io/intro-to-templates-p3-functions/
// Changes from original post are explained in comments.
import (
	"html/template"
	"log"
	"math/rand" // Changed, create a new fake user.ID every request.
	"net/http"
)

var testTemplate *template.Template

type ViewData struct {
	User User
}

type User struct {
	ID    int
	Email string
}

func main() {
	var err error
	testTemplate, err = template.New("hello.gohtml").Funcs(template.FuncMap{
		"hasPermission": func(feature string) bool {
			return false
		},
	}).ParseFiles("hello.gohtml")
	if err != nil {
		panic(err)
	}

	http.HandleFunc("/", handler)
	http.ListenAndServe(":3000", nil)
}

func handler(w http.ResponseWriter, r *http.Request) {
	w.Header().Set("Content-Type", "text/html")
	// Object "loaded" from database.
	user := User{
		ID:    rand.Intn(10000), // Changed.
		Email: "jon@calhoun.io",
	}

	callCount := 0 // Added, close over an additional object to make data access conflict clear.

	vd := ViewData{user}
	// testTemplate is a single object. Calls to Funcs() change the global FuncMap
	// and therefore what variables have been closed over.
	err := testTemplate.Funcs(template.FuncMap{
		"hasPermission": func(feature string) bool {
			callCount++ // Added, count the number of function calls using a closure.
			if callCount > 2 { // Added, the template calls hasPermission() twice.
				log.Fatal("Called more than twice") // Added, Fatal when called more than twice.
			}
			if user.ID == 1 && feature == "feature-a" {
				return true
			}
			return false
		},
	}).Execute(w, vd) // Template calls hasPermission(), but the function may change at anytime.
	if err != nil {
		http.Error(w, err.Error(), http.StatusInternalServerError)
	}
}

terminal1-go-run.txt

# Start the application and run ab in a separate terminal.

$ go run main.go 
2019/01/03 09:17:23 Called more than twice
exit status 1

terminal2-ab-pass.txt

# No load from single client. Requests are ordered, data access is ordered.
# Test can also be done with seige or any other concurrent load tester.

$ ab -c 1 -n 500 http://localhost:3000/
This is ApacheBench, Version 2.3 <$Revision: 1807734 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
Completed 100 requests
Completed 200 requests
Completed 300 requests
Completed 400 requests
Completed 500 requests
Finished 500 requests


Server Software:        
Server Hostname:        localhost
Server Port:            3000

Document Path:          /
Document Length:        431 bytes

Concurrency Level:      1
Time taken for tests:   0.086 seconds
Complete requests:      500
Failed requests:        51
   (Connect: 0, Receive: 0, Length: 51, Exceptions: 0)
Total transferred:      266443 bytes
HTML transferred:       215443 bytes
Requests per second:    5807.07 [#/sec] (mean)
Time per request:       0.172 [ms] (mean)
Time per request:       0.172 [ms] (mean, across all concurrent requests)
Transfer rate:          3021.98 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:     0    0   0.0      0       1
Waiting:        0    0   0.0      0       1
Total:          0    0   0.1      0       1

Percentage of the requests served within a certain time (ms)
  50%      0
  66%      0
  75%      0
  80%      0
  90%      0
  95%      0
  98%      0
  99%      0
 100%      1 (longest request)
 

terminal3-ab-fail.txt

# Under load, requests are constant and data access is random.
# Application Fatals.

$ ab -c 100 -n 500 http://localhost:3000/
This is ApacheBench, Version 2.3 <$Revision: 1807734 $>
Copyright 1996 Adam Twiss, Zeus Technology Ltd, http://www.zeustech.net/
Licensed to The Apache Software Foundation, http://www.apache.org/

Benchmarking localhost (be patient)
apr_socket_recv: Connection reset by peer (104)
Total of 5 requests completed