Merge pull request #1752 from JessonChan/ab_lock_race

fix template  read-write lock race

config.go

@@ -16,7 +16,6 @@ package beego
 
 import (
 	"fmt"
-	"html/template"
 	"os"
 	"path/filepath"
 	"strings"
@@ -106,8 +105,6 @@ var (
 	AppConfig *beegoAppConfig
 	// AppPath is the absolute path to the app
 	AppPath string
-	// TemplateCache stores template caching
-	TemplateCache map[string]*template.Template
 	// GlobalSessions is the instance for the session manager
 	GlobalSessions *session.Manager
 

controller.go

@@ -197,16 +197,40 @@ func (c *Controller) RenderString() (string, error) {
 
 // RenderBytes returns the bytes of rendered template string. Do not send out response.
 func (c *Controller) RenderBytes() ([]byte, error) {
-	//if the controller has set layout, then first get the tplname's content set the content to the layout
+	buf, err := c.renderTemplate()
+	//if the controller has set layout, then first get the tplName's content set the content to the layout
+	if err == nil && c.Layout != "" {
+		c.Data["LayoutContent"] = template.HTML(buf.String())
+
+		if c.LayoutSections != nil {
+			for sectionName, sectionTpl := range c.LayoutSections {
+				if sectionTpl == "" {
+					c.Data[sectionName] = ""
+					continue
+				}
+				buf.Reset()
+				err = executeTemplate(&buf, sectionTpl, c.Data)
+				if err != nil {
+					return nil, err
+				}
+				c.Data[sectionName] = template.HTML(buf.String())
+			}
+		}
+
+		buf.Reset()
+		executeTemplate(&buf, c.Layout, c.Data)
+	}
+	return buf.Bytes(), err
+}
+
+func (c *Controller) renderTemplate() (bytes.Buffer, error) {
 	var buf bytes.Buffer
-	if c.Layout != "" {
 	if c.TplName == "" {
 		c.TplName = strings.ToLower(c.controllerName) + "/" + strings.ToLower(c.actionName) + "." + c.TplExt
 	}
-
 	if BConfig.RunMode == DEV {
 		buildFiles := []string{c.TplName}
-			if c.LayoutSections != nil {
+		if c.Layout != "" && c.LayoutSections != nil {
 			for _, sectionTpl := range c.LayoutSections {
 				if sectionTpl == "" {
 					continue
@@ -216,58 +240,7 @@ func (c *Controller) RenderBytes() ([]byte, error) {
 		}
 		BuildTemplate(BConfig.WebConfig.ViewsPath, buildFiles...)
 	}
-		if _, ok := BeeTemplates[c.TplName]; !ok {
+	return buf, executeTemplate(&buf, c.TplName, c.Data)
-			panic("can't find templatefile in the path:" + c.TplName)
-		}
-		err := BeeTemplates[c.TplName].ExecuteTemplate(&buf, c.TplName, c.Data)
-		if err != nil {
-			Trace("template Execute err:", err)
-			return nil, err
-		}
-		c.Data["LayoutContent"] = template.HTML(buf.String())
-
-		if c.LayoutSections != nil {
-			for sectionName, sectionTpl := range c.LayoutSections {
-				if sectionTpl == "" {
-					c.Data[sectionName] = ""
-					continue
-				}
-
-				buf.Reset()
-				err = BeeTemplates[sectionTpl].ExecuteTemplate(&buf, sectionTpl, c.Data)
-				if err != nil {
-					Trace("template Execute err:", err)
-					return nil, err
-				}
-				c.Data[sectionName] = template.HTML(buf.String())
-			}
-		}
-
-		buf.Reset()
-		err = BeeTemplates[c.Layout].ExecuteTemplate(&buf, c.Layout, c.Data)
-		if err != nil {
-			Trace("template Execute err:", err)
-			return nil, err
-		}
-		return buf.Bytes(), nil
-	}
-
-	if c.TplName == "" {
-		c.TplName = strings.ToLower(c.controllerName) + "/" + strings.ToLower(c.actionName) + "." + c.TplExt
-	}
-	if BConfig.RunMode == DEV {
-		BuildTemplate(BConfig.WebConfig.ViewsPath, c.TplName)
-	}
-	if _, ok := BeeTemplates[c.TplName]; !ok {
-		panic("can't find templatefile in the path:" + c.TplName)
-	}
-	buf.Reset()
-	err := BeeTemplates[c.TplName].ExecuteTemplate(&buf, c.TplName, c.Data)
-	if err != nil {
-		Trace("template Execute err:", err)
-		return nil, err
-	}
-	return buf.Bytes(), nil
 }
 
 // Redirect sends the redirection response to url with status code.

template.go

@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
+	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -30,13 +31,28 @@ import (
 
 var (
 	beegoTplFuncMap = make(template.FuncMap)
-	// BeeTemplates caching map and supported template file extensions.
+	// beeTemplates caching map and supported template file extensions.
-	BeeTemplates  = make(map[string]*template.Template)
+	beeTemplates  = make(map[string]*template.Template)
-	templatesLock sync.Mutex
+	templatesLock sync.RWMutex
-	// BeeTemplateExt stores the template extension which will build
+	// beeTemplateExt stores the template extension which will build
-	BeeTemplateExt = []string{"tpl", "html"}
+	beeTemplateExt = []string{"tpl", "html"}
 )
 
+func executeTemplate(wr io.Writer, name string, data interface{}) error {
+	if BConfig.RunMode == DEV {
+		templatesLock.RLock()
+		defer templatesLock.RUnlock()
+	}
+	if t, ok := beeTemplates[name]; ok {
+		err := t.ExecuteTemplate(wr, name, data)
+		if err != nil {
+			Trace("template Execute err:", err)
+		}
+		return err
+	}
+	panic("can't find templatefile in the path:" + name)
+}
+
 func init() {
 	beegoTplFuncMap["dateformat"] = DateFormat
 	beegoTplFuncMap["date"] = Date
@@ -55,7 +71,6 @@ func init() {
 	beegoTplFuncMap["config"] = GetConfig
 	beegoTplFuncMap["map_get"] = MapGet
 
-	// go1.2 added template funcs
 	// Comparisons
 	beegoTplFuncMap["eq"] = eq // ==
 	beegoTplFuncMap["ge"] = ge // >=
@@ -103,7 +118,7 @@ func (tf *templateFile) visit(paths string, f os.FileInfo, err error) error {
 
 // HasTemplateExt return this path contains supported template extension of beego or not.
 func HasTemplateExt(paths string) bool {
-	for _, v := range BeeTemplateExt {
+	for _, v := range beeTemplateExt {
 		if strings.HasSuffix(paths, "."+v) {
 			return true
 		}
@@ -113,12 +128,12 @@ func HasTemplateExt(paths string) bool {
 
 // AddTemplateExt add new extension for template.
 func AddTemplateExt(ext string) {
-	for _, v := range BeeTemplateExt {
+	for _, v := range beeTemplateExt {
 		if v == ext {
 			return
 		}
 	}
-	BeeTemplateExt = append(BeeTemplateExt, ext)
+	beeTemplateExt = append(beeTemplateExt, ext)
 }
 
 // BuildTemplate will build all template files in a directory.
@@ -149,7 +164,7 @@ func BuildTemplate(dir string, files ...string) error {
 				if err != nil {
 					Trace("parse template err:", file, err)
 				} else {
-					BeeTemplates[file] = t
+					beeTemplates[file] = t
 				}
 				templatesLock.Unlock()
 			}

template_test.go

@@ -70,10 +70,10 @@ func TestTemplate(t *testing.T) {
 	if err := BuildTemplate(dir); err != nil {
 		t.Fatal(err)
 	}
-	if len(BeeTemplates) != 3 {
+	if len(beeTemplates) != 3 {
-		t.Fatalf("should be 3 but got %v", len(BeeTemplates))
+		t.Fatalf("should be 3 but got %v", len(beeTemplates))
 	}
-	if err := BeeTemplates["index.tpl"].ExecuteTemplate(os.Stdout, "index.tpl", nil); err != nil {
+	if err := beeTemplates["index.tpl"].ExecuteTemplate(os.Stdout, "index.tpl", nil); err != nil {
 		t.Fatal(err)
 	}
 	for _, name := range files {
@@ -126,7 +126,7 @@ func TestRelativeTemplate(t *testing.T) {
 	if err := BuildTemplate(dir, files[1]); err != nil {
 		t.Fatal(err)
 	}
-	if err := BeeTemplates["easyui/rbac/user.tpl"].ExecuteTemplate(os.Stdout, "easyui/rbac/user.tpl", nil); err != nil {
+	if err := beeTemplates["easyui/rbac/user.tpl"].ExecuteTemplate(os.Stdout, "easyui/rbac/user.tpl", nil); err != nil {
 		t.Fatal(err)
 	}
 	for _, name := range files {