fix for #107 that doesn't require a mutex

I didn't look into why the mutex was causing some sort of deadlock condition
on retrieving points. This fixes it, as long as the underlying filesystem
supports atomic file renames. So don't run this with NFS.

src/award.go

@@ -14,6 +14,24 @@ type Award struct {
 	Points   int
 }
 
+type AwardList []*Award
+
+// Implement sort.Interface on AwardList
+func (awards AwardList) Len() int {
+  return len(awards)
+}
+
+func (awards AwardList) Less(i, j int) bool {
+  return awards[i].When.Before(awards[j].When)
+}
+
+func (awards AwardList) Swap(i, j int) {
+  tmp := awards[i]
+  awards[i] = awards[j]
+  awards[j] = tmp
+}
+
+
 func ParseAward(s string) (*Award, error) {
 	ret := Award{}
 

src/award_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"testing"
+	"sort"
 )
 
 func TestAward(t *testing.T) {
@@ -32,3 +33,23 @@ func TestAward(t *testing.T) {
 		t.Error("Not throwing error on bad points")
 	}
 }
+
+func TestAwardList(t *testing.T) {
+  a, _ := ParseAward("1536958399 1a2b3c4d counting 1")
+  b, _ := ParseAward("1536958400 1a2b3c4d counting 1")
+  c, _ := ParseAward("1536958300 1a2b3c4d counting 1")
+  list := AwardList{a, b, c}
+  
+  if sort.IsSorted(list) {
+    t.Error("Unsorted list thinks it's sorted")
+  }
+  
+  sort.Stable(list)
+  if (list[0] != c) || (list[1] != a) || (list[2] != b) {
+    t.Error("Sorting didn't")
+  }
+  
+  if ! sort.IsSorted(list) {
+    t.Error("Sorted list thinks it isn't")
+  }
+}

src/instance.go

@@ -37,7 +37,6 @@ type Instance struct {
 	nextAttempt       map[string]time.Time
 	nextAttemptMutex  *sync.RWMutex
 	mux               *http.ServeMux
-	PointsMux         *sync.RWMutex
 }
 
 func (ctx *Instance) Initialize() error {
@@ -55,7 +54,6 @@ func (ctx *Instance) Initialize() error {
 	ctx.nextAttempt = map[string]time.Time{}
 	ctx.nextAttemptMutex = new(sync.RWMutex)
 	ctx.mux = http.NewServeMux()
-	ctx.PointsMux = new(sync.RWMutex)
 
 	ctx.BindHandlers()
 	ctx.MaybeInitialize()
@@ -85,10 +83,7 @@ func (ctx *Instance) MaybeInitialize() {
 	// Remove any extant control and state files
 	os.Remove(ctx.StatePath("until"))
 	os.Remove(ctx.StatePath("disabled"))
-
-	ctx.PointsMux.Lock()
 	os.Remove(ctx.StatePath("points.log"))
-	ctx.PointsMux.Unlock()
 
 	os.RemoveAll(ctx.StatePath("points.tmp"))
 	os.RemoveAll(ctx.StatePath("points.new"))
@@ -158,18 +153,15 @@ func (ctx *Instance) TooFast(teamId string) bool {
 	return now.Before(next)
 }
 
-func (ctx *Instance) PointsLog(teamId string) []*Award {
-	var ret []*Award
-
+func (ctx *Instance) PointsLog(teamId string) AwardList {
+	awardlist := AwardList{}
+	
 	fn := ctx.StatePath("points.log")
 
-	ctx.PointsMux.RLock()
-	defer ctx.PointsMux.RUnlock()
-
 	f, err := os.Open(fn)
 	if err != nil {
 		log.Printf("Unable to open %s: %s", fn, err)
-		return ret
+		return awardlist
 	}
 	defer f.Close()
 
@@ -184,10 +176,10 @@ func (ctx *Instance) PointsLog(teamId string) []*Award {
 		if len(teamId) > 0 && cur.TeamId != teamId {
 			continue
 		}
-		ret = append(ret, cur)
+		awardlist = append(awardlist, cur)
 	}
 
-	return ret
+	return awardlist
 }
 
 // AwardPoints gives points to teamId in category.

src/maintenance.go

@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
 	"log"
 	"os"
@@ -205,20 +204,26 @@ func (ctx *Instance) readTeams() {
 // collectPoints gathers up files in points.new/ and appends their contents to points.log,
 // removing each points.new/ file as it goes.
 func (ctx *Instance) collectPoints() {
-	ctx.PointsMux.Lock()
-	defer ctx.PointsMux.Unlock()
+  points := ctx.PointsLog("")
 
-	logf, err := os.OpenFile(ctx.StatePath("points.log"), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+  pointsFilename := ctx.StatePath("points.log")
+  pointsNewFilename := ctx.StatePath("points.log.new")
+  
+  // Yo, this is delicate.
+  // If we have to return early, we must remove this file.
+  // If the file's written and we move it successfully,
+  // we need to remove all the little points files that built it.
+	newPoints, err := os.OpenFile(pointsNewFilename, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0644)
 	if err != nil {
 		log.Printf("Can't append to points log: %s", err)
 		return
 	}
-	defer logf.Close()
 
 	files, err := ioutil.ReadDir(ctx.StatePath("points.new"))
 	if err != nil {
 		log.Printf("Error reading packages: %s", err)
 	}
+	removearino := make([]string, 0, len(files))
 	for _, f := range files {
 		filename := ctx.StatePath("points.new", f.Name())
 		s, err := ioutil.ReadFile(filename)
@@ -233,7 +238,7 @@ func (ctx *Instance) collectPoints() {
 		}
 
 		duplicate := false
-		for _, e := range ctx.PointsLog("") {
+		for _, e := range points {
 			if award.Same(e) {
 				duplicate = true
 				break
@@ -243,53 +248,30 @@ func (ctx *Instance) collectPoints() {
 		if duplicate {
 			log.Printf("Skipping duplicate points: %s", award.String())
 		} else {
-			fmt.Fprintf(logf, "%s\n", award.String())
+		  points = append(points, award)
 		}
-
-		logf.Sync()
-		if err := os.Remove(filename); err != nil {
-			log.Printf("Unable to remove %s: %s", filename, err)
-		}
-	}
-}
-
-// Ensure that points.log is sorted chronologically
-func (ctx *Instance) sortPoints() {
-	var points []*Award
-
-	ctx.PointsMux.Lock()
-	defer ctx.PointsMux.Unlock()
-
-	logf, err := os.OpenFile(ctx.StatePath("points.log"), os.O_CREATE|os.O_RDWR, 0644)
-	if err != nil {
-		log.Printf("Can't sort points.log: %s", err)
-		return
-	}
-	defer logf.Close()
-
-	scanner := bufio.NewScanner(logf)
-	for scanner.Scan() {
-		line := scanner.Text()
-		cur, err := ParseAward(line)
-		if err != nil {
-			log.Printf("Encountered malformed award line, not sorting %s: %s", line, err)
-			return
-		}
-
-		points = append(points, cur)
+		removearino = append(removearino, filename)
 	}
 
-	// Only sort and write to file if we need to
-	if ! sort.SliceIsSorted(points, func( i, j int) bool { return points[i].When.Before(points[j].When) }) {
+	sort.Stable(points)
+	for _, point := range points {
+		fmt.Fprintln(newPoints, point.String())
+	}
+	
+	newPoints.Close()
+	
+	if err := os.Rename(pointsNewFilename, pointsFilename); err != nil {
+	  log.Printf("Unable to move %s to %s: %s", pointsFilename, pointsNewFilename, err)
+	  if err := os.Remove(pointsNewFilename); err != nil {
+	    log.Printf("Also couldn't remove %s: %s", pointsNewFilename, err)
+	  }
+	  return
+	}
 
-		sort.SliceStable(points, func(i, j int) bool { return points[i].When.Before(points[j].When) })
-
-		logf.Seek(0, io.SeekStart)
-
-		for i := range points {
-			point := points[i]
-			fmt.Fprintf(logf, "%s\n", point.String())
-		}
+	for _, filename := range removearino {
+  	if err := os.Remove(filename); err != nil {
+  		log.Printf("Unable to remove %s: %s", filename, err)
+  	}
 	}
 }
 
@@ -338,7 +320,6 @@ func (ctx *Instance) Maintenance(maintenanceInterval time.Duration) {
 			ctx.tidy()
 			ctx.readTeams()
 			ctx.collectPoints()
-			ctx.sortPoints()
 			ctx.generatePuzzleList()
 			ctx.generatePointsLog("")
 		}