From 720ec2bd315af6cd8b1a09a5dbed5b70c53e4d14 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <brad@danga.com>
Date: Thu, 4 Sep 2014 08:26:58 -0700
Subject: [PATCH] importer: fix start-up race
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The status handler was trying to call methods on importer before
the *importer.Host had been initialized:

==================
WARNING: DATA RACE
Read by goroutine 31:
  camlistore.org/pkg/importer.(*importer).Accounts()
      /Users/bradfitz/src/camlistore.org/pkg/importer/importer.go:827 +0x344
  camlistore.org/pkg/importer.(*Host).AccountsStatus()
      /Users/bradfitz/src/camlistore.org/pkg/importer/importer.go:360 +0x1ea
  camlistore.org/pkg/server.(*StatusHandler).currentStatus()
      /Users/bradfitz/src/camlistore.org/pkg/server/status.go:166 +0x910
  camlistore.org/pkg/server.func·007()
      /Users/bradfitz/src/camlistore.org/pkg/server/status.go:73 +0x6e
Previous write by goroutine 15:
  camlistore.org/pkg/importer.(*Host).InitHandler()
      /Users/bradfitz/src/camlistore.org/pkg/importer/importer.go:402 +0x2d3
  camlistore.org/pkg/serverinit.(*Config).InstallHandlers()
      /Users/bradfitz/src/camlistore.org/pkg/serverinit/serverinit.go:583 +0x10b7
  camlistore.org/server/camlistored.Main()
      /Users/bradfitz/src/camlistore.org/server/camlistored/camlistored.go:427 +0xa94
Goroutine 31 (running) created at:
  camlistore.org/pkg/server.(*StatusHandler).InitHandler()
      /Users/bradfitz/src/camlistore.org/pkg/server/status.go:83 +0x207
  camlistore.org/pkg/serverinit.(*Config).InstallHandlers()
      /Users/bradfitz/src/camlistore.org/pkg/serverinit/serverinit.go:583 +0x10b7
  camlistore.org/server/camlistored.Main()
      /Users/bradfitz/src/camlistore.org/server/camlistored/camlistored.go:427 +0xa94
Goroutine 15 (running) created at:
  camlistore.org/server/camlistored.TestStarts()
      /Users/bradfitz/src/camlistore.org/server/camlistored/run_test.go:70 +0xc81
  testing.tRunner()
      /Users/bradfitz/go/src/pkg/testing/testing.go:427 +0x112

Change-Id: I5f4c7aaeb15f5b99e574a419c959b01e58b431af
---
 pkg/importer/importer.go | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go
index f96e62ed9..51c95d801 100644
--- a/pkg/importer/importer.go
+++ b/pkg/importer/importer.go
@@ -231,7 +231,12 @@ func newFromConfig(ld blobserver.Loader, cfg jsonconfig.Obj) (http.Handler, erro
 	}
 	hc.ClientId = ClientId
 	hc.ClientSecret = ClientSecret
-	return NewHost(hc)
+	host, err := NewHost(hc)
+	if err != nil {
+		return nil, err
+	}
+	host.didInit.Add(1)
+	return host, nil
 }
 
 var _ blobserver.HandlerIniter = (*Host)(nil)
@@ -333,6 +338,14 @@ type Host struct {
 	signer       *schema.Signer
 	uiPrefix     string // or empty if no UI handler
 
+	// didInit is incremented by newFromConfig and marked done
+	// after InitHandler. Any method on Host that requires Init
+	// then calls didInit.Wait to guard against initialization
+	// races where serverinit calls InitHandler in a random
+	// order on start-up and different handlers access the
+	// not-yet-initialized Host (notably from a goroutine)
+	didInit sync.WaitGroup
+
 	// HTTPClient optionally specifies how to fetch external network
 	// resources. Defaults to http.DefaultClient.
 	client    *http.Client
@@ -353,6 +366,7 @@ type accountStatus struct {
 // AccountsStatus returns the currently configured accounts and their status for
 // inclusion in the status.json document, as rendered by the web UI.
 func (h *Host) AccountsStatus() (interface{}, []camtypes.StatusError) {
+	h.didInit.Wait()
 	var s []accountStatus
 	var errs []camtypes.StatusError
 	for _, impName := range h.importers {
@@ -413,6 +427,7 @@ func (h *Host) InitHandler(hl blobserver.FindHandlerByTyper) error {
 	if h.signer == nil {
 		return errors.New("importer requires a 'jsonsign' handler")
 	}
+	h.didInit.Done()
 	go h.startPeriodicImporters()
 	return nil
 }