Merge lp:~chipaca/ubuntu-push/flapping-fixes into lp:ubuntu-push/automatic

[John Lenton]

Partially work around the issue in a minimally intrusive way (real fix will have to wait).

Partially work around the issue in a minimally intrusive way (real fix will have to wait).

[Bret Barker (noise)]

LGTM

review: Approve

[Ubuntu One Auto Pilot (otto-pilot)]

The attempt to merge lp:~chipaca/ubuntu-push/flapping-fixes into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

scripts/deps.sh ubuntu-push-client.go
scripts/deps.sh server/dev/server.go
scripts/deps.sh server/acceptance/cmd/acceptanceclient.go
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -t launchpad.net/ubuntu-push launchpad.net/ubuntu-push/accounts launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/emblemcounter launchpad.net/ubuntu-push/bus/haptic launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/polld launchpad.net/ubuntu-push/bus/powerd launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/unitygreeter launchpad.net/ubuntu-push/bus/windowstack launchpad.net/ubuntu-push/click launchpad.net/ubuntu-push/click/cappinfo launchpad.net/ubuntu-push/click/cblacklist launchpad.net/ubuntu-push/click/cclick launchpad.net/ubuntu-push/click/testing launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/service launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/seenstate launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/identifier launchpad.net/ubuntu-push/identifier/testing launchpad.net/ubuntu-push/launch_helper launchpad.net/ubuntu-push/launch_helper/cual launchpad.net/ubuntu-push/launch_helper/helper_finder launchpad.net/ubuntu-push/launch_helper/legacy launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/messaging launchpad.net/ubuntu-push/messaging/cmessaging launchpad.net/ubuntu-push/messaging/reply launchpad.net/ubuntu-push/nih launchpad.net/ubuntu-push/nih/cnih launchpad.net/ubuntu-push/poller launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/sounds launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/urldispatcher launchpad.net/ubuntu-push/urldispatcher/curldispatcher launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/server/acceptance/cmd/ launchpad.net/ubuntu-push/server/dev/ launchpad.net/ubuntu-push/ 2>/dev/null | cat > dependencies.tsv
rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 code.google.com/p/go-uuid/uuid
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
code.google.com/p/go-uuid now at 7dda39b2e7d5e26...

[lp:~chipaca/ubuntu-push/flapping-fixes]

360. By John Lenton on 2015-01-22

fix a massive data race in session

=== modified file 'bus/connectivity/connectivity.go'
--- bus/connectivity/connectivity.go	2015-01-22 10:38:04 +0000
+++ bus/connectivity/connectivity.go	2015-01-22 10:38:04 +0000
@@ -132,13 +132,17 @@
 				return false, errors.New("got not-OK from StateChanged watch")
 			}
 			cs.webgetCh = nil
-			cs.currentState = v
-			cs.timer.Reset(stabilizingTimeout)
-			log.Debugf("State changed to %s. Assuming disconnect.", v)
-			if cs.lastSent == true {
-				log.Infof("Sending 'disconnected'.")
-				cs.lastSent = false
-				break Loop
+			if v != networkmanager.Connecting && cs.currentState != v {
+				cs.currentState = v
+				cs.timer.Reset(stabilizingTimeout)
+				log.Debugf("state changed to %s. Assuming disconnect.", v)
+				if cs.lastSent == true {
+					log.Debugf("sending 'disconnected'.")
+					cs.lastSent = false
+					break Loop
+				}
+			} else {
+				log.Debugf("got State of %s, current is %s, ignoring.", v, cs.currentState)
 			}
 
 		case <-cs.timer.C:

=== modified file 'bus/connectivity/connectivity_test.go'
--- bus/connectivity/connectivity_test.go	2014-07-04 23:00:42 +0000
+++ bus/connectivity/connectivity_test.go	2015-01-22 10:38:04 +0000
@@ -217,20 +217,22 @@
 	f, e := cs.connectedStateStep()
 	c.Check(e, IsNil)
 	c.Check(f, Equals, true)
-	ch <- networkmanager.ConnectedGlobal // a ConnectedGlobal when connected signals trouble
-	f, e = cs.connectedStateStep()
-	c.Check(e, IsNil)
-	c.Check(f, Equals, false) // so we assume a disconnect happened
-	f, e = cs.connectedStateStep()
-	c.Check(e, IsNil)
-	c.Check(f, Equals, true) // and if the web check works, go back to connected
+	ch <- networkmanager.Disconnected
+	ch <- networkmanager.ConnectedGlobal
+	f, e = cs.connectedStateStep()
+	c.Check(e, IsNil)
+	c.Check(f, Equals, false)
+	f, e = cs.connectedStateStep()
+	c.Check(e, IsNil)
+	c.Check(f, Equals, true)
 
 	// same scenario, but with failing web check
 	webget_p = condition.Fail2Work(1)
+	ch <- networkmanager.Disconnected
 	ch <- networkmanager.ConnectedGlobal
 	f, e = cs.connectedStateStep()
 	c.Check(e, IsNil)
-	c.Check(f, Equals, false) // first false is from assuming a Connected signals trouble
+	c.Check(f, Equals, false) // first false is from the Disconnected
 
 	// the next call to Step will time out
 	_ch := make(chan bool, 1)
@@ -284,7 +286,6 @@
 
 	endp := testingbus.NewTestingEndpoint(condition.Work(true), condition.Work(true),
 		uint32(networkmanager.ConnectedGlobal),
-		uint32(networkmanager.ConnectedGlobal),
 		uint32(networkmanager.Disconnected),
 	)
 
@@ -303,8 +304,6 @@
 	}{
 		{false, "first state is always false", 0},
 		{true, "then it should be true as per ConnectedGlobal above", 0},
-		{false, "then, false (upon receiving the next ConnectedGlobal)", 2},
-		{true, "then it should be true (webcheck passed)", 0},
 		{false, "then it should be false (Disconnected)", 2},
 		{false, "then it should be false again because it's restarted", 2},
 	}

=== modified file 'bus/endpoint.go'
--- bus/endpoint.go	2015-01-21 17:21:42 +0000
+++ bus/endpoint.go	2015-01-22 10:38:04 +0000
@@ -84,7 +84,7 @@
 	name := endp.addr.Name
 	hasOwner, err := d.NameHasOwner(name)
 	if err != nil {
-		endp.log.Debugf("Unable to determine ownership of %#v: %v", name, err)
+		endp.log.Debugf("unable to determine ownership of %#v: %v", name, err)
 		bus.Close()
 		return err
 	}
@@ -126,7 +126,7 @@
 func (endp *endpoint) WatchSignal(member string, f func(...interface{}), d func()) error {
 	watch, err := endp.proxy.WatchSignal(endp.addr.Interface, member)
 	if err != nil {
-		endp.log.Debugf("Failed to set up the watch: %s", err)
+		endp.log.Debugf("failed to set up the watch: %s", err)
 		return err
 	}
 
@@ -167,15 +167,15 @@
 	variantvs := endp.unpackOneMsg(msg, property)
 	switch len(variantvs) {
 	default:
-		return nil, fmt.Errorf("Too many values in Properties.Get response: %d", len(variantvs))
+		return nil, fmt.Errorf("too many values in Properties.Get response: %d", len(variantvs))
 	case 0:
-		return nil, fmt.Errorf("Not enough values in Properties.Get response: %d", len(variantvs))
+		return nil, fmt.Errorf("not enough values in Properties.Get response: %d", len(variantvs))
 	case 1:
 		// carry on
 	}
 	variant, ok := variantvs[0].(*dbus.Variant)
 	if !ok {
-		return nil, fmt.Errorf("Response from Properties.Get wasn't a *dbus.Variant")
+		return nil, fmt.Errorf("response from Properties.Get wasn't a *dbus.Variant")
 	}
 	return variant.Value, nil
 }
@@ -324,6 +324,6 @@
 		}
 		f(endp.unpackOneMsg(msg, member)...)
 	}
-	endp.log.Errorf("Got not-OK from %s watch", member)
+	endp.log.Errorf("got not-OK from %s watch", member)
 	d()
 }

=== modified file 'bus/networkmanager/networkmanager.go'
--- bus/networkmanager/networkmanager.go	2014-04-04 12:01:42 +0000
+++ bus/networkmanager/networkmanager.go	2015-01-22 10:38:04 +0000
@@ -71,14 +71,14 @@
 func (nm *networkManager) GetState() State {
 	s, err := nm.bus.GetProperty("state")
 	if err != nil {
-		nm.log.Errorf("Failed gettting current state: %s", err)
-		nm.log.Debugf("Defaulting state to Unknown")
+		nm.log.Errorf("failed gettting current state: %s", err)
+		nm.log.Debugf("defaulting state to Unknown")
 		return Unknown
 	}
 
 	v, ok := s.(uint32)
 	if !ok {
-		nm.log.Errorf("Got weird state: %#v", s)
+		nm.log.Errorf("got weird state: %#v", s)
 		return Unknown
 	}
 
@@ -110,8 +110,8 @@
 func (nm *networkManager) GetPrimaryConnection() string {
 	s, err := nm.bus.GetProperty("PrimaryConnection")
 	if err != nil {
-		nm.log.Errorf("Failed gettting current primary connection: %s", err)
-		nm.log.Debugf("Defaulting primary connection to empty")
+		nm.log.Errorf("failed gettting current primary connection: %s", err)
+		nm.log.Debugf("defaulting primary connection to empty")
 		return ""
 	}
 
@@ -146,7 +146,7 @@
 			ch <- string(con)
 		}, func() { close(ch) })
 	if err != nil {
-		nm.log.Debugf("Failed to set up the watch: %s", err)
+		nm.log.Debugf("failed to set up the watch: %s", err)
 		return nil, err
 	}
 

=== modified file 'bus/notifications/raw.go'
--- bus/notifications/raw.go	2014-08-15 10:33:04 +0000
+++ bus/notifications/raw.go	2015-01-22 10:38:04 +0000
@@ -119,7 +119,7 @@
 			ch <- action
 		}, func() { close(ch) })
 	if err != nil {
-		raw.log.Debugf("Failed to set up the watch: %s", err)
+		raw.log.Debugf("failed to set up the watch: %s", err)
 		return nil, err
 	}
 	return ch, nil

=== modified file 'client/client.go'
--- client/client.go	2014-11-03 13:36:00 +0000
+++ client/client.go	2015-01-22 10:38:04 +0000
@@ -381,10 +381,9 @@
 		return
 	}
 	client.hasConnectivity = hasConnectivity
+	client.session.Close()
 	if hasConnectivity {
 		client.session.AutoRedial(client.sessionConnectedCh)
-	} else {
-		client.session.Close()
 	}
 }
 
@@ -490,7 +489,7 @@
 		case err := <-client.session.ErrCh:
 			errhandler(err)
 		case count := <-client.sessionConnectedCh:
-			client.log.Debugf("Session connected after %d attempts", count)
+			client.log.Debugf("session connected after %d attempts", count)
 		case app := <-client.unregisterCh:
 			unregisterhandler(app)
 		}

=== modified file 'client/client_test.go'
--- client/client_test.go	2014-09-05 10:47:29 +0000
+++ client/client_test.go	2015-01-22 10:38:04 +0000
@@ -1135,7 +1135,7 @@
 	// sessionConnectedCh to nothing in particular, but it'll help sync this test
 	cli.sessionConnectedCh <- 42
 	tick()
-	c.Check(cs.log.Captured(), Matches, "(?ms).*Session connected after 42 attempts$")
+	c.Check(cs.log.Captured(), Matches, "(?msi).*Session connected after 42 attempts$")
 
 	// loop() should have connected:
 	//  * connCh to the connectivity checker

=== modified file 'client/service/service.go'
--- client/service/service.go	2014-11-19 13:58:12 +0000
+++ client/service/service.go	2015-01-22 10:38:04 +0000
@@ -149,14 +149,14 @@
 	// errors below here Can't Happen (tm).
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		svc.Log.Errorf("Reading response body: %v", err)
+		svc.Log.Errorf("during ReadAll() of response body: %v", err)
 		return nil, err
 	}
 
 	var reply registrationReply
 	err = json.Unmarshal(body, &reply)
 	if err != nil {
-		svc.Log.Errorf("Unmarshalling response body: %v", err)
+		svc.Log.Errorf("during Unmarshal of response body: %v", err)
 		return nil, fmt.Errorf("unable to unmarshal register response: %v", err)
 	}
 
@@ -181,7 +181,7 @@
 	}
 
 	if !reply.Ok || reply.Token == "" {
-		svc.Log.Errorf("Unexpected response: %#v", reply)
+		svc.Log.Errorf("unexpected response: %#v", reply)
 		return nil, ErrBadToken
 	}
 

=== modified file 'client/session/session.go'
--- client/session/session.go	2015-01-21 17:21:42 +0000
+++ client/session/session.go	2015-01-22 10:38:04 +0000
@@ -74,8 +74,22 @@
 	Connected
 	Started
 	Running
+	Unknown
 )
 
+func (s ClientSessionState) String() string {
+	if s >= Unknown {
+		return fmt.Sprintf("??? (%d)", s)
+	}
+	return [Unknown]string{
+		"Error",
+		"Disconnected",
+		"Connected",
+		"Started",
+		"Running",
+	}[s]
+}
+
 type hostGetter interface {
 	Get() (*gethosts.Host, error)
 }
@@ -131,6 +145,7 @@
 	proto        protocol.Protocol
 	pingInterval time.Duration
 	retrier      util.AutoRedialer
+	retrierLock  sync.Mutex
 	cookie       string
 	// status
 	stateP          *uint32
@@ -348,6 +363,8 @@
 }
 
 func (sess *ClientSession) stopRedial() {
+	sess.retrierLock.Lock()
+	defer sess.retrierLock.Unlock()
 	if sess.retrier != nil {
 		sess.retrier.Stop()
 		sess.retrier = nil
@@ -360,15 +377,30 @@
 		sess.setShouldDelay()
 	}
 	time.Sleep(sess.redialDelay(sess))
+	sess.retrierLock.Lock()
+	defer sess.retrierLock.Unlock()
+	if sess.retrier != nil {
+		panic("session AutoRedial: unexpected non-nil retrier.")
+	}
 	sess.retrier = util.NewAutoRedialer(sess)
 	sess.lastAutoRedial = time.Now()
-	go func() { doneCh <- sess.retrier.Redial() }()
+	go func() {
+		sess.retrierLock.Lock()
+		retrier := sess.retrier
+		sess.retrierLock.Unlock()
+		if retrier == nil {
+			sess.Log.Debugf("session autoredialer skipping retry: retrier has been set to nil.")
+			return
+		}
+		doneCh <- retrier.Redial()
+	}()
 }
 
 func (sess *ClientSession) Close() {
 	sess.stopRedial()
 	sess.doClose()
 }
+
 func (sess *ClientSession) doClose() {
 	sess.connLock.Lock()
 	defer sess.connLock.Unlock()
@@ -593,7 +625,7 @@
 	}
 	sess.proto = proto
 	sess.pingInterval = pingInterval
-	sess.Log.Debugf("Connected %v.", conn.RemoteAddr())
+	sess.Log.Debugf("connected %v.", conn.RemoteAddr())
 	sess.started() // deals with choosing which host to retry with as well
 	return nil
 }

=== modified file 'identifier/identifier.go'
--- identifier/identifier.go	2014-08-04 20:40:50 +0000
+++ identifier/identifier.go	2015-01-22 10:38:04 +0000
@@ -48,7 +48,7 @@
 func New() (Id, error) {
 	value, err := readMachineId()
 	if err != nil {
-		return &Identifier{value: ""}, fmt.Errorf("Failed to read the machine id: %s", err)
+		return &Identifier{value: ""}, fmt.Errorf("failed to read the machine id: %s", err)
 	}
 	return &Identifier{value: value}, nil
 }

=== modified file 'identifier/identifier_test.go'
--- identifier/identifier_test.go	2014-08-12 00:32:32 +0000
+++ identifier/identifier_test.go	2015-01-22 10:38:04 +0000
@@ -48,7 +48,7 @@
 	machineIdPath = "/var/lib/dbus/no-such-file"
 	id, err := New()
 	c.Check(err, NotNil)
-	c.Check(err.Error(), Equals, "Failed to read the machine id: open /var/lib/dbus/no-such-file: no such file or directory")
+	c.Check(err.Error(), Equals, "failed to read the machine id: open /var/lib/dbus/no-such-file: no such file or directory")
 	c.Check(id.String(), HasLen, 0)
 }
 

=== modified file 'launch_helper/helper_finder/helper_finder.go'
--- launch_helper/helper_finder/helper_finder.go	2014-07-29 15:36:00 +0000
+++ launch_helper/helper_finder/helper_finder.go	2015-01-22 10:38:04 +0000
@@ -72,7 +72,7 @@
 	fInfo, err := os.Stat(helpersDataPath)
 	if err != nil {
 		// cache file is missing, go via the slow route
-		log.Infof("Cache file not found, falling back to .json file lookup")
+		log.Infof("cache file not found, falling back to .json file lookup")
 		return helperFromHookFile(app)
 	}
 	// get the lock as the map can be changed while we read

=== modified file 'launch_helper/helper_finder/helper_finder_test.go'
--- launch_helper/helper_finder/helper_finder_test.go	2014-07-29 15:36:00 +0000
+++ launch_helper/helper_finder/helper_finder_test.go	2015-01-22 10:38:04 +0000
@@ -139,7 +139,7 @@
 	hid, hex := Helper(app, s.log)
 	c.Check(hid, Equals, "com.example.test_test-helper_1")
 	c.Check(hex, Equals, filepath.Join(s.symlinkPath, "tsthlpr"))
-	c.Check(s.log.Captured(), Matches, ".*Cache file not found, falling back to .json file lookup\n")
+	c.Check(s.log.Captured(), Matches, ".*(?i)Cache file not found, falling back to .json file lookup\n")
 }
 
 func (s *helperSuite) TestHelperFromHookBasic(c *C) {

=== modified file 'launch_helper/legacy/legacy.go'
--- launch_helper/legacy/legacy.go	2014-11-05 12:07:53 +0000
+++ launch_helper/legacy/legacy.go	2015-01-22 10:38:04 +0000
@@ -78,7 +78,7 @@
 		p_err := cmd.Wait()
 		if p_err != nil {
 			// Helper failed or got killed, log output/errors
-			lhl.log.Errorf("Legacy helper failed: appId: %v, helper: %v, pid: %v, error: %v, stdout: %#v, stderr: %#v.",
+			lhl.log.Errorf("legacy helper failed: appId: %v, helper: %v, pid: %v, error: %v, stdout: %#v, stderr: %#v.",
 				appId, progname, id, p_err, stdout.String(), stderr.String())
 		}
 		lhl.done(id)

=== modified file 'launch_helper/legacy/legacy_test.go'
--- launch_helper/legacy/legacy_test.go	2014-08-21 18:03:49 +0000
+++ launch_helper/legacy/legacy_test.go	2015-01-22 10:38:04 +0000
@@ -104,7 +104,7 @@
 	c.Assert(err, IsNil)
 
 	takeNext(ch, c)
-	c.Check(ls.log.Captured(), Matches, "(?s).*Legacy helper failed.*")
+	c.Check(ls.log.Captured(), Matches, "(?si).*Legacy helper failed.*")
 }
 
 func (ls *legacySuite) TestHelperFailsLog(c *C) {

=== modified file 'messaging/messaging.go'
--- messaging/messaging.go	2014-07-27 02:54:40 +0000
+++ messaging/messaging.go	2015-01-22 10:38:04 +0000
@@ -181,7 +181,7 @@
 			Action:   action,
 		})
 		if err != nil {
-			mmu.Log.Errorf("Failed to build action: %s", action)
+			mmu.Log.Errorf("failed to build action: %s", action)
 			return false
 		}
 		actions[2*i] = string(act)

=== modified file 'testing/helpers.go'
--- testing/helpers.go	2014-07-11 19:42:57 +0000
+++ testing/helpers.go	2015-01-22 10:38:04 +0000
@@ -118,7 +118,7 @@
 
 		idx := strings.LastIndex(dir, sep)
 		if idx == -1 {
-			panic(fmt.Errorf("Unable to find %s in %#v", sep, dir))
+			panic(fmt.Errorf("unable to find %s in %#v", sep, dir))
 		}
 		idx += len(sep)
 

=== modified file 'urldispatcher/curldispatcher/curldispatcher.go'
--- urldispatcher/curldispatcher/curldispatcher.go	2014-09-01 13:27:17 +0000
+++ urldispatcher/curldispatcher/curldispatcher.go	2015-01-22 10:38:04 +0000
@@ -79,7 +79,7 @@
 	C.dispatch_url(c_url, (C.gpointer)(&payload))
 	success := <-doneCh
 	if !success {
-		return fmt.Errorf("Failed to DispatchURL: %s for %s", url, appPackage)
+		return fmt.Errorf("failed to DispatchURL: %s for %s", url, appPackage)
 	}
 	return nil
 }

=== modified file 'urldispatcher/urldispatcher.go'
--- urldispatcher/urldispatcher.go	2014-09-01 13:27:17 +0000
+++ urldispatcher/urldispatcher.go	2015-01-22 10:38:04 +0000
@@ -44,7 +44,7 @@
 var cTestURL = curldispatcher.TestURL
 
 func (ud *urlDispatcher) DispatchURL(url string, app *click.AppId) error {
-	ud.log.Debugf("Dispatching %s", url)
+	ud.log.Debugf("dispatching %s", url)
 	err := cDispatchURL(url, app.DispatchPackage())
 	if err != nil {
 		ud.log.Errorf("DispatchURL failed: %s", err)
@@ -62,7 +62,7 @@
 	}
 	for _, appId := range appIds {
 		if appId != app.Versioned() {
-			ud.log.Debugf("Notification skipped because of different appid for actions: %v - %s != %s", urls, appId, app.Versioned())
+			ud.log.Debugf("notification skipped because of different appid for actions: %v - %s != %s", urls, appId, app.Versioned())
 			return false
 		}
 	}

=== modified file 'urldispatcher/urldispatcher_test.go'
--- urldispatcher/urldispatcher_test.go	2014-08-21 17:45:01 +0000
+++ urldispatcher/urldispatcher_test.go	2015-01-22 10:38:04 +0000
@@ -106,7 +106,7 @@
 	appId := clickhelp.MustParseAppId("com.example.test_app_0.99")
 	urls := []string{"potato://test-app"}
 	c.Check(ud.TestURL(appId, urls), Equals, false)
-	c.Check(s.log.Captured(), Matches, `(?sm).*Notification skipped because of different appid for actions: \[potato://test-app\] - com.example.test_test-app_0.1 != com.example.test_app_0.99`)
+	c.Check(s.log.Captured(), Matches, `(?smi).*notification skipped because of different appid for actions: \[potato://test-app\] - com.example.test_test-app_0.1 != com.example.test_app_0.99`)
 }
 
 func (s *UDSuite) TestTestURLOneWrongApp(c *C) {
@@ -117,7 +117,7 @@
 	appId := clickhelp.MustParseAppId("com.example.test_test-app_0")
 	urls := []string{"potato://test-app", "potato_a://foo"}
 	c.Check(ud.TestURL(appId, urls), Equals, false)
-	c.Check(s.log.Captured(), Matches, `(?sm).*Notification skipped because of different appid for actions: \[potato://test-app potato_a://foo\] - com.example.test_test-app1 != com.example.test_test-app.*`)
+	c.Check(s.log.Captured(), Matches, `(?smi).*notification skipped because of different appid for actions: \[potato://test-app potato_a://foo\] - com.example.test_test-app1 != com.example.test_test-app.*`)
 }
 
 func (s *UDSuite) TestTestURLInvalidURL(c *C) {

=== modified file 'util/redialer.go'
--- util/redialer.go	2014-03-20 12:15:47 +0000
+++ util/redialer.go	2015-01-22 10:38:04 +0000
@@ -19,6 +19,7 @@
 
 import (
 	"sync"
+	"sync/atomic"
 	"time"
 )
 
@@ -65,30 +66,42 @@
 	Stop()          // Stop shuts down the given AutoRedialer, if it is still retrying.
 }
 
+type redialerState uint32
+
+const (
+	Unconfigured redialerState = iota
+	Redialing
+	Stopped
+)
+
+func (s *redialerState) String() string {
+	return [3]string{"Unconfigured", "Redialing", "Stopped"}[uint32(*s)]
+}
+
 type autoRedialer struct {
-	stop   chan bool
-	lock   sync.RWMutex
+	stateP *redialerState
 	dial   func() error
 	jitter func(time.Duration) time.Duration
 }
 
+func (ar *autoRedialer) state() redialerState {
+	return redialerState(atomic.LoadUint32((*uint32)(ar.stateP)))
+}
+
+func (ar *autoRedialer) setState(s redialerState) {
+	atomic.StoreUint32((*uint32)(ar.stateP), uint32(s))
+}
+
+func (ar *autoRedialer) setStateIfEqual(oldState, newState redialerState) bool {
+	return atomic.CompareAndSwapUint32((*uint32)(ar.stateP), uint32(oldState), uint32(newState))
+}
+
 func (ar *autoRedialer) Stop() {
 	if ar != nil {
-		ar.lock.RLock()
-		defer ar.lock.RUnlock()
-		if ar.stop != nil {
-			ar.stop <- true
-		}
+		ar.setState(Stopped)
 	}
 }
 
-func (ar *autoRedialer) shutdown() {
-	ar.lock.Lock()
-	defer ar.lock.Unlock()
-	close(ar.stop)
-	ar.stop = nil
-}
-
 // Redial keeps on calling Dial until it stops returning an error.  It does
 // exponential backoff, adding back the output of Jitter at each step.
 func (ar *autoRedialer) Redial() uint32 {
@@ -96,20 +109,19 @@
 		// at least it's better than a segfault...
 		panic("you can't Redial a nil AutoRedialer")
 	}
-	if ar.stop == nil {
-		panic("this AutoRedialer has already been shut down")
+	if !ar.setStateIfEqual(Unconfigured, Redialing) {
+		// XXX log this
+		return 0
 	}
-	defer ar.shutdown()
-
-	ar.lock.RLock()
-	stop := ar.stop
-	ar.lock.RUnlock()
 
 	var timeout time.Duration
 	var dialAttempts uint32 = 0 // unsigned so it can wrap safely ...
 	timeouts := Timeouts()
 	var numTimeouts uint32 = uint32(len(timeouts))
 	for {
+		if ar.state() != Redialing {
+			return dialAttempts
+		}
 		if ar.dial() == nil {
 			return dialAttempts + 1
 		}
@@ -122,18 +134,15 @@
 			timeout += ar.jitter(timeout)
 		}
 		dialAttempts++
-		select {
-		case <-stop:
-			return dialAttempts
-		case <-time.After(timeout):
-		}
+		time.Sleep(timeout)
 	}
 }
 
 // Returns a stoppable AutoRedialer using the provided Dialer. If the Dialer
 // is also a Jitterer, the backoff will be jittered.
 func NewAutoRedialer(dialer Dialer) AutoRedialer {
-	ar := &autoRedialer{stop: make(chan bool), dial: dialer.Dial}
+	state := Unconfigured
+	ar := &autoRedialer{stateP: &state, dial: dialer.Dial}
 	jitterer, ok := dialer.(Jitterer)
 	if ok {
 		ar.jitter = jitterer.Jitter

=== added file 'util/redialer_states.gv'
--- util/redialer_states.gv	1970-01-01 00:00:00 +0000
+++ util/redialer_states.gv	2015-01-22 10:38:04 +0000
@@ -0,0 +1,9 @@
+digraph "redialer" {
+    "Unconfigured" -> "Redialing" [ label="Redial" ]
+    "Unconfigured" -> "Stopped"   [ label="Stop"   ]
+
+    "Redialing"    -> "Redialing" [ label="Redial" ]
+    "Redialing"    -> "Stopped"   [ label="Stop"   ]
+
+    "Stopped"      -> "Stopped"   [ label="*"      ]
+}

=== modified file 'util/redialer_test.go'
--- util/redialer_test.go	2014-02-05 13:02:47 +0000
+++ util/redialer_test.go	2015-01-22 10:38:04 +0000
@@ -48,10 +48,10 @@
 func (s *RedialerSuite) TestWorks(c *C) {
 	endp := testibus.NewTestingEndpoint(condition.Fail2Work(3), nil)
 	ar := NewAutoRedialer(endp)
-	c.Check(ar.(*autoRedialer).stop, NotNil)
+	//	c.Check(ar.(*autoRedialer).stop, NotNil)
 	c.Check(ar.Redial(), Equals, uint32(4))
 	// and on success, the stopper goes away
-	c.Check(ar.(*autoRedialer).stop, IsNil)
+	//	c.Check(ar.(*autoRedialer).stop, IsNil)
 }
 
 func (s *RedialerSuite) TestRetryNil(c *C) {
@@ -63,7 +63,7 @@
 	endp := testibus.NewTestingEndpoint(condition.Work(true), nil)
 	ar := NewAutoRedialer(endp)
 	c.Check(ar.Redial(), Equals, uint32(1))
-	c.Check(ar.Redial, PanicMatches, ".*shut.?down.*")
+	c.Check(ar.Redial(), Equals, uint32(0))
 }
 
 type JitteringEndpoint struct {
@@ -103,13 +103,13 @@
 	go func() { countCh <- ar.Redial() }()
 	ar.Stop()
 	select {
-	case n := <-countCh:
-		c.Check(n, Equals, uint32(1))
+	case <-countCh:
+		// pass
 	case <-time.After(20 * time.Millisecond):
 		c.Fatal("timed out waiting for redial")
 	}
-	// on Stop(), the stopper goes away too
-	c.Check(ar.(*autoRedialer).stop, IsNil)
+	// on Stop(), the redialer is Stopped
+	c.Check(ar.(*autoRedialer).state(), Equals, Stopped)
 	// and the next Stop() doesn't panic nor block
 	ar.Stop()
 }