Merge pull request #1800 from calmh/ursvc

Break out usage reporting into a service

cmd/syncthing/gui.go

@@ -485,22 +485,16 @@ func (s *apiSvc) postSystemConfig(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// Start or stop usage reporting as appropriate
+	// Fixup usage reporting settings
 
 	if curAcc := cfg.Options().URAccepted; newCfg.Options.URAccepted > curAcc {
 		// UR was enabled
 		newCfg.Options.URAccepted = usageReportVersion
 		newCfg.Options.URUniqueID = randomString(8)
-		err := sendUsageReport(s.model)
-		if err != nil {
-			l.Infoln("Usage report:", err)
-		}
-		go usageReportingLoop(s.model)
 	} else if newCfg.Options.URAccepted < curAcc {
 		// UR was disabled
 		newCfg.Options.URAccepted = -1
 		newCfg.Options.URUniqueID = ""
-		stopUsageReporting()
 	}
 
 	// Activate and save

cmd/syncthing/main.go

@@ -677,16 +677,13 @@ func syncthingMain() {
 			cfg.SetOptions(opts)
 			cfg.Save()
 		}
-		go usageReportingLoop(m)
-		go func() {
-			time.Sleep(10 * time.Minute)
-			err := sendUsageReport(m)
-			if err != nil {
-				l.Infoln("Usage report:", err)
-			}
-		}()
 	}
 
+	// The usageReportingManager registers itself to listen to configuration
+	// changes, and there's nothing more we need to tell it from the outside.
+	// Hence we don't keep the returned pointer.
+	newUsageReportingManager(m, cfg)
+
 	if opts.RestartOnWakeup {
 		go standbyMonitor()
 	}

cmd/syncthing/usage_report.go

@@ -16,7 +16,9 @@ import (
 	"runtime"
 	"time"
 
+	"github.com/syncthing/syncthing/internal/config"
 	"github.com/syncthing/syncthing/internal/model"
+	"github.com/thejerf/suture"
 )
 
 // Current version number of the usage report, for acceptance purposes. If
@@ -24,8 +26,45 @@ import (
 // are prompted for acceptance of the new report.
 const usageReportVersion = 1
 
-var stopUsageReportingCh = make(chan struct{})
+type usageReportingManager struct {
+	model *model.Model
+	sup   *suture.Supervisor
+}
 
+func newUsageReportingManager(m *model.Model, cfg *config.Wrapper) *usageReportingManager {
+	mgr := &usageReportingManager{
+		model: m,
+	}
+
+	// Start UR if it's enabled.
+	mgr.Changed(cfg.Raw())
+
+	// Listen to future config changes so that we can start and stop as
+	// appropriate.
+	cfg.Subscribe(mgr)
+
+	return mgr
+}
+
+func (m *usageReportingManager) Changed(cfg config.Configuration) error {
+	if cfg.Options.URAccepted >= usageReportVersion && m.sup == nil {
+		// Usage reporting was turned on; lets start it.
+		svc := &usageReportingService{
+			model: m.model,
+		}
+		m.sup = suture.NewSimple("usageReporting")
+		m.sup.Add(svc)
+		m.sup.ServeBackground()
+	} else if cfg.Options.URAccepted < usageReportVersion && m.sup != nil {
+		// Usage reporting was turned off
+		m.sup.Stop()
+		m.sup = nil
+	}
+	return nil
+}
+
+// reportData returns the data to be sent in a usage report. It's used in
+// various places, so not part of the usageReportingSvc object.
 func reportData(m *model.Model) map[string]interface{} {
 	res := make(map[string]interface{})
 	res["uniqueID"] = cfg.Options().URUniqueID
@@ -75,8 +114,13 @@ func reportData(m *model.Model) map[string]interface{} {
 	return res
 }
 
-func sendUsageReport(m *model.Model) error {
+type usageReportingService struct {
-	d := reportData(m)
+	model *model.Model
+	stop  chan struct{}
+}
+
+func (s *usageReportingService) sendUsageReport() error {
+	d := reportData(s.model)
 	var b bytes.Buffer
 	json.NewEncoder(&b).Encode(d)
 
@@ -94,32 +138,32 @@ func sendUsageReport(m *model.Model) error {
 	return err
 }
 
-func usageReportingLoop(m *model.Model) {
+func (s *usageReportingService) Serve() {
+	s.stop = make(chan struct{})
+
 	l.Infoln("Starting usage reporting")
-	t := time.NewTicker(86400 * time.Second)
+	defer l.Infoln("Stopping usage reporting")
-loop:
+
+	t := time.NewTimer(10 * time.Minute) // time to initial report at start
 	for {
 		select {
-		case <-stopUsageReportingCh:
+		case <-s.stop:
-			break loop
+			return
 		case <-t.C:
-			err := sendUsageReport(m)
+			err := s.sendUsageReport()
 			if err != nil {
 				l.Infoln("Usage report:", err)
 			}
+			t.Reset(24 * time.Hour) // next report tomorrow
 		}
 	}
-	l.Infoln("Stopping usage reporting")
-}
-
-func stopUsageReporting() {
-	select {
-	case stopUsageReportingCh <- struct{}{}:
-	default:
-	}
 }
 
-// Returns CPU performance as a measure of single threaded SHA-256 MiB/s
+func (s *usageReportingService) Stop() {
+	close(s.stop)
+}
+
+// cpuBench returns CPU performance as a measure of single threaded SHA-256 MiB/s
 func cpuBench() float64 {
 	chunkSize := 100 * 1 << 10
 	h := sha256.New()