Code cleanups.

Fix more race conditions.

middleware/markdown/markdown.go

@@ -4,13 +4,14 @@ package markdown
 
 import (
 	"io/ioutil"
+	"log"
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 
 	"github.com/mholt/caddy/middleware"
 	"github.com/russross/blackfriday"
-	//	"log"
 )
 
 // Markdown implements a layer of middleware that serves
@@ -70,11 +71,14 @@ type Config struct {
 
 	// Directory to store static files
 	StaticDir string
+
+	sync.RWMutex
 }
 
 // ServeHTTP implements the http.Handler interface.
 func (md Markdown) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
-	for _, m := range md.Configs {
+	for i := range md.Configs {
+		m := &md.Configs[i]
 		if !middleware.Path(r.URL.Path).Matches(m.PathScope) {
 			continue
 		}
@@ -120,11 +124,9 @@ func (md Markdown) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error
 
 				if m.StaticDir != "" {
 					// Markdown modified or new. Update links.
-					//					go func() {
+					if err := GenerateLinks(md, m); err != nil {
-					//						if err := GenerateLinks(md, &md.Configs[i]); err != nil {
+						log.Println(err)
-					//							log.Println(err)
+					}
-					//						}
-					//					}()
 				}
 
 				body, err := ioutil.ReadAll(f)
@@ -137,7 +139,7 @@ func (md Markdown) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error
 					Req:  r,
 					URL:  r.URL,
 				}
-				html, err := md.Process(m, fpath, body, ctx)
+				html, err := md.Process(*m, fpath, body, ctx)
 				if err != nil {
 					return http.StatusInternalServerError, err
 				}

middleware/markdown/markdown_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/mholt/caddy/middleware"
@@ -18,20 +19,24 @@ func TestMarkdown(t *testing.T) {
 		FileSys: http.Dir("./testdata"),
 		Configs: []Config{
 			Config{
-				Renderer:   blackfriday.HtmlRenderer(0, "", ""),
+				Renderer:    blackfriday.HtmlRenderer(0, "", ""),
-				PathScope:  "/blog",
+				PathScope:   "/blog",
-				Extensions: []string{".md"},
+				Extensions:  []string{".md"},
-				Styles:     []string{},
+				Styles:      []string{},
-				Scripts:    []string{},
+				Scripts:     []string{},
-				Templates:  templates,
+				Templates:   templates,
+				StaticDir:   DefaultStaticDir,
+				StaticFiles: make(map[string]string),
 			},
 			Config{
-				Renderer:   blackfriday.HtmlRenderer(0, "", ""),
+				Renderer:    blackfriday.HtmlRenderer(0, "", ""),
-				PathScope:  "/log",
+				PathScope:   "/log",
-				Extensions: []string{".md"},
+				Extensions:  []string{".md"},
-				Styles:     []string{"/resources/css/log.css", "/resources/css/default.css"},
+				Styles:      []string{"/resources/css/log.css", "/resources/css/default.css"},
-				Scripts:    []string{"/resources/js/log.js", "/resources/js/default.js"},
+				Scripts:     []string{"/resources/js/log.js", "/resources/js/default.js"},
-				Templates:  make(map[string]string),
+				Templates:   make(map[string]string),
+				StaticDir:   DefaultStaticDir,
+				StaticFiles: make(map[string]string),
 			},
 		},
 		IndexFiles: []string{"index.html"},
@@ -123,4 +128,34 @@ func getTrue() bool {
 	if respBody != expectedBody {
 		t.Fatalf("Expected body: %v got: %v", expectedBody, respBody)
 	}
+
+	expectedLinks := []string{
+		"/blog/test.md",
+		"/log/test.md",
+	}
+
+	for i, c := range md.Configs {
+		if c.Links[0].Url != expectedLinks[i] {
+			t.Fatalf("Expected %v got %v", expectedLinks[i], c.Links[0].Url)
+		}
+	}
+
+	// attempt to trigger race condition
+	var w sync.WaitGroup
+	f := func() {
+		req, err := http.NewRequest("GET", "/log/test.md", nil)
+		if err != nil {
+			t.Fatalf("Could not create HTTP request: %v", err)
+		}
+		rec := httptest.NewRecorder()
+
+		md.ServeHTTP(rec, req)
+		w.Done()
+	}
+	for i := 0; i < 5; i++ {
+		w.Add(1)
+		go f()
+	}
+	w.Wait()
+
 }

middleware/markdown/page.go

@@ -12,17 +12,15 @@ import (
 	"github.com/russross/blackfriday"
 )
 
-var (
-	pagesMutex      sync.RWMutex
-	linksGenerating bool
-)
-
 const (
+	// Date format YYYY-MM-DD HH:MM:SS
 	timeLayout = `2006-01-02 15:04:05`
+
+	// Length of page summary.
 	summaryLen = 150
 )
 
-// Page represents a statically generated markdown page.
+// PageLink represents a statically generated markdown page.
 type PageLink struct {
 	Title   string
 	Summary string
@@ -30,25 +28,51 @@ type PageLink struct {
 	Url     string
 }
 
-// pageLinkSorter sort PageLink by newest date to oldest.
+// pageLinkSorter sorts PageLink by newest date to oldest.
 type pageLinkSorter []PageLink
 
 func (p pageLinkSorter) Len() int           { return len(p) }
 func (p pageLinkSorter) Swap(i, j int)      { p[i], p[j] = p[j], p[i] }
 func (p pageLinkSorter) Less(i, j int) bool { return p[i].Date.After(p[j].Date) }
 
-func GenerateLinks(md Markdown, cfg *Config) error {
+type linkGen struct {
-	if linksGenerating {
+	generating bool
-		return nil
+	waiters    int
-	}
+	lastErr    error
+	sync.RWMutex
+	sync.WaitGroup
+}
 
-	pagesMutex.Lock()
+func (l *linkGen) addWaiter() {
-	linksGenerating = true
+	l.WaitGroup.Add(1)
+	l.waiters++
+}
+
+func (l *linkGen) discardWaiters() {
+	l.Lock()
+	defer l.Unlock()
+	for i := 0; i < l.waiters; i++ {
+		l.Done()
+	}
+}
+
+func (l *linkGen) started() bool {
+	l.RLock()
+	defer l.RUnlock()
+	return l.generating
+}
+
+func (l *linkGen) generateLinks(md Markdown, cfg *Config) {
+	l.Lock()
+	l.generating = true
+	l.Unlock()
 
 	fp := filepath.Join(md.Root, cfg.PathScope)
 
 	cfg.Links = []PageLink{}
-	err := filepath.Walk(fp, func(path string, info os.FileInfo, err error) error {
+
+	cfg.Lock()
+	l.lastErr = filepath.Walk(fp, func(path string, info os.FileInfo, err error) error {
 		for _, ext := range cfg.Extensions {
 			if !info.IsDir() && strings.HasSuffix(info.Name(), ext) {
 				// Load the file
@@ -92,12 +116,46 @@ func GenerateLinks(md Markdown, cfg *Config) error {
 			}
 		}
 
-		// sort by newest date
-		sort.Sort(pageLinkSorter(cfg.Links))
 		return nil
 	})
 
-	linksGenerating = false
+	// sort by newest date
-	pagesMutex.Unlock()
+	sort.Sort(pageLinkSorter(cfg.Links))
-	return err
+	cfg.Unlock()
+
+	l.Lock()
+	l.generating = false
+	l.Unlock()
+}
+
+type linkGenerator struct {
+	gens map[*Config]*linkGen
+	sync.Mutex
+}
+
+var generator = linkGenerator{gens: make(map[*Config]*linkGen)}
+
+// GenerateLinks generates links to all markdown files ordered by newest date.
+// This blocks until link generation is done. When called by multiple goroutines,
+// the first caller starts the generation and others only wait.
+func GenerateLinks(md Markdown, cfg *Config) error {
+	generator.Lock()
+
+	// if link generator exists for config and running, wait.
+	if g, ok := generator.gens[cfg]; ok {
+		if g.started() {
+			g.addWaiter()
+			generator.Unlock()
+			g.Wait()
+			return g.lastErr
+		}
+	}
+
+	g := &linkGen{}
+	generator.gens[cfg] = g
+	generator.Unlock()
+
+	g.generateLinks(md, cfg)
+	g.discardWaiters()
+	return g.lastErr
 }

middleware/markdown/process.go

@@ -101,9 +101,9 @@ func (md Markdown) processTemplate(c Config, requestPath string, tmpl []byte, me
 		Links:   c.Links,
 	}
 
-	pagesMutex.RLock()
+	c.RLock()
 	err = t.Execute(b, mdData)
-	pagesMutex.RUnlock()
+	c.RUnlock()
 
 	if err != nil {
 		return nil, err