Merge lp:~pedronis/ubuntu-push/automatic-land-to-vivid into lp:ubuntu-push

[Samuele Pedroni]

[Roberto Alsina]
* click-hook: report failure if hooks_path doesn't exist. [client]

[Bret Barker]
* add a hacky busy sleep loop to workaround go's sleep not
  accounting for suspended time, more logging (lp:1435109). [client]

[John R. Lenton]
* Refactor code maintaining session (better fix for lp:1390663) [client]

[Samuele Pedroni]
* just delegate whether there's a update-worthy image to the
  system-settings helper and system-image. [client]
* stop waking up for polling if in flight-mode and wireless not
  enabled (lp:1437135). [client]
* don't hold a lock for a long time on handleErrConn, trigger
  autoRedial on Error more actively (lp:1435109). [client]
* disallow RC4 and SSLv3. [server]

[Roberto Alsina]
* click-hook: report failure if hooks_path doesn't exist. [client]

[Bret Barker]
* add a hacky busy sleep loop to workaround go's sleep not
  accounting for suspended time, more logging (lp:1435109). [client]

[John R. Lenton]
* Refactor code maintaining session (better fix for lp:1390663) [client]

[Samuele Pedroni]
* just delegate whether there's a update-worthy image to the
  system-settings helper and system-image. [client]
* stop waking up for polling if in flight-mode and wireless not
  enabled (lp:1437135). [client]
* don't hold a lock for a long time on handleErrConn, trigger
  autoRedial on Error more actively (lp:1435109). [client]
* disallow RC4 and SSLv3. [server]

[Roberto Alsina (ralsina)]

review: Approve

=== modified file 'bus/networkmanager/networkmanager.go'
--- bus/networkmanager/networkmanager.go	2015-02-26 19:36:57 +0000
+++ bus/networkmanager/networkmanager.go	2015-04-02 09:52:21 +0000
@@ -1,5 +1,5 @@
 /*
- Copyright 2013-2014 Canonical Ltd.
+ Copyright 2013-2015 Canonical Ltd.
 
  This program is free software: you can redistribute it and/or modify it
  under the terms of the GNU General Public License version 3, as published
@@ -14,9 +14,10 @@
  with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-// Package networkmanager wraps a couple of NetworkManager's DBus API points:
-// the org.freedesktop.NetworkManager.state call, and listening for the
-// StateChange signal.
+// Package networkmanager wraps a couple of NetworkManager's DBus API
+// points: the org.freedesktop.NetworkManager.state call, and
+// listening for the StateChange signal, similarly for the primary
+// connection and wireless enabled state.
 package networkmanager
 
 import (
@@ -47,8 +48,14 @@
 	// primary connection.
 	GetPrimaryConnection() string
 	// WatchPrimaryConnection listens for changes of NetworkManager's
-	// Primary Connection, and sends it out over the channel returned.
+	// Primary Connection, and sends them out over the channel returned.
 	WatchPrimaryConnection() (<-chan string, bus.Cancellable, error)
+	// GetWirelessEnabled fetches and returns NetworkManager's
+	// wireless state.
+	GetWirelessEnabled() bool
+	// WatchWirelessEnabled listens for changes of NetworkManager's
+	// wireless state, and sends them out over the channel returned.
+	WatchWirelessEnabled() (<-chan bool, bus.Cancellable, error)
 }
 
 type networkManager struct {
@@ -71,7 +78,7 @@
 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.Errorf("failed getting current state: %s", err)
 		nm.log.Debugf("defaulting state to Unknown")
 		return Unknown
 	}
@@ -108,16 +115,16 @@
 }
 
 func (nm *networkManager) GetPrimaryConnection() string {
-	s, err := nm.bus.GetProperty("PrimaryConnection")
+	got, 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 getting current PrimaryConnection: %s", err)
+		nm.log.Debugf("defaulting PrimaryConnection to empty")
 		return ""
 	}
 
-	v, ok := s.(dbus.ObjectPath)
+	v, ok := got.(dbus.ObjectPath)
 	if !ok {
-		nm.log.Errorf("got weird PrimaryConnection: %#v", s)
+		nm.log.Errorf("got weird PrimaryConnection: %#v", got)
 		return ""
 	}
 
@@ -142,7 +149,7 @@
 				nm.log.Errorf("got weird PrimaryConnection via PropertiesChanged: %#v", v)
 				return
 			}
-			nm.log.Debugf("got primary connection: %s", con)
+			nm.log.Debugf("got PrimaryConnection change: %s", con)
 			ch <- string(con)
 		}, func() { close(ch) })
 	if err != nil {
@@ -152,3 +159,49 @@
 
 	return ch, w, nil
 }
+
+func (nm *networkManager) GetWirelessEnabled() bool {
+	got, err := nm.bus.GetProperty("WirelessEnabled")
+	if err != nil {
+		nm.log.Errorf("failed getting WirelessEnabled: %s", err)
+		nm.log.Debugf("defaulting WirelessEnabled to true")
+		return true
+	}
+
+	v, ok := got.(bool)
+	if !ok {
+		nm.log.Errorf("got weird WirelessEnabled: %#v", got)
+		return true
+	}
+
+	return v
+}
+
+func (nm *networkManager) WatchWirelessEnabled() (<-chan bool, bus.Cancellable, error) {
+	ch := make(chan bool)
+	w, err := nm.bus.WatchSignal("PropertiesChanged",
+		func(ppsi ...interface{}) {
+			pps, ok := ppsi[0].(map[string]dbus.Variant)
+			if !ok {
+				nm.log.Errorf("got weird PropertiesChanged: %#v", ppsi[0])
+				return
+			}
+			v, ok := pps["WirelessEnabled"]
+			if !ok {
+				return
+			}
+			en, ok := v.Value.(bool)
+			if !ok {
+				nm.log.Errorf("got weird WirelessEnabled via PropertiesChanged: %#v", v)
+				return
+			}
+			nm.log.Debugf("got WirelessEnabled change: %v", en)
+			ch <- en
+		}, func() { close(ch) })
+	if err != nil {
+		nm.log.Debugf("failed to set up the watch: %s", err)
+		return nil, nil, err
+	}
+
+	return ch, w, nil
+}

=== modified file 'bus/networkmanager/networkmanager_test.go'
--- bus/networkmanager/networkmanager_test.go	2015-02-26 19:36:57 +0000
+++ bus/networkmanager/networkmanager_test.go	2015-04-02 09:52:21 +0000
@@ -1,5 +1,5 @@
 /*
- Copyright 2013-2014 Canonical Ltd.
+ Copyright 2013-2015 Canonical Ltd.
 
  This program is free software: you can redistribute it and/or modify it
  under the terms of the GNU General Public License version 3, as published
@@ -232,3 +232,111 @@
 	c.Check(ok, Equals, true)
 	c.Check(v, Equals, "42")
 }
+
+// GetWirelessEnabled returns the right state when everything works
+func (s *NMSuite) TestGetWirelessEnabled(c *C) {
+	nm := New(testingbus.NewTestingEndpoint(nil, condition.Work(true), false), s.log)
+	en := nm.GetWirelessEnabled()
+	c.Check(en, Equals, false)
+}
+
+// GetWirelessEnabled returns the right state when dbus fails
+func (s *NMSuite) TestGetWirelessEnabledFail(c *C) {
+	nm := New(testingbus.NewTestingEndpoint(nil, condition.Work(false)), s.log)
+	en := nm.GetWirelessEnabled()
+	c.Check(en, Equals, true)
+}
+
+// GetWirelessEnabled returns the right state when dbus works but delivers rubbish values
+func (s *NMSuite) TestGetWirelessEnabledRubbishValues(c *C) {
+	nm := New(testingbus.NewTestingEndpoint(nil, condition.Work(true), "broken"), s.log)
+	en := nm.GetWirelessEnabled()
+	c.Check(en, Equals, true)
+}
+
+// GetWirelessEnabled returns the right state when dbus works but delivers a rubbish structure
+func (s *NMSuite) TestGetWirelessEnabledRubbishStructure(c *C) {
+	nm := New(testingbus.NewMultiValuedTestingEndpoint(nil, condition.Work(true), []interface{}{}), s.log)
+	en := nm.GetWirelessEnabled()
+	c.Check(en, Equals, true)
+}
+
+func mkWirelessEnMap(en bool) map[string]dbus.Variant {
+	m := make(map[string]dbus.Variant)
+	m["WirelessEnabled"] = dbus.Variant{en}
+	return m
+}
+
+// WatchWirelessEnabled sends a stream of wireless enabled states over the channel
+func (s *NMSuite) TestWatchWirelessEnabled(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true),
+		mkWirelessEnMap(true),
+		mkWirelessEnMap(false),
+		mkWirelessEnMap(true),
+	)
+	nm := New(tc, s.log)
+	ch, w, err := nm.WatchWirelessEnabled()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	l := []bool{<-ch, <-ch, <-ch}
+	c.Check(l, DeepEquals, []bool{true, false, true})
+}
+
+// WatchWirelessEnabled returns on error if the dbus call fails
+func (s *NMSuite) TestWatchWirelessEnabledFails(c *C) {
+	nm := New(testingbus.NewTestingEndpoint(nil, condition.Work(false)), s.log)
+	_, _, err := nm.WatchWirelessEnabled()
+	c.Check(err, NotNil)
+}
+
+// WatchWirelessEnabled calls close on its channel when the watch bails
+func (s *NMSuite) TestWatchWirelessEnabledClosesOnWatchBail(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true))
+	nm := New(tc, s.log)
+	ch, w, err := nm.WatchWirelessEnabled()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	_, ok := <-ch
+	c.Check(ok, Equals, false)
+}
+
+// WatchWirelessEnabled survives rubbish values
+func (s *NMSuite) TestWatchWirelessEnabledSurvivesRubbishValues(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true), "gorp")
+	nm := New(tc, s.log)
+	ch, w, err := nm.WatchWirelessEnabled()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	_, ok := <-ch
+	c.Check(ok, Equals, false)
+}
+
+// WatchWirelessEnabled ignores non-WirelessEnabled PropertyChanged
+func (s *NMSuite) TestWatchWirelessEnabledIgnoresIrrelephant(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true),
+		map[string]dbus.Variant{"foo": dbus.Variant{}},
+		map[string]dbus.Variant{"WirelessEnabled": dbus.Variant{true}},
+	)
+	nm := New(tc, s.log)
+	ch, w, err := nm.WatchWirelessEnabled()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	v, ok := <-ch
+	c.Check(ok, Equals, true)
+	c.Check(v, Equals, true)
+}
+
+// WatchWirelessEnabled ignores rubbish WirelessEnabled
+func (s *NMSuite) TestWatchWirelessEnabledIgnoresRubbishValues(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true),
+		map[string]dbus.Variant{"WirelessEnabled": dbus.Variant{-12}},
+		map[string]dbus.Variant{"WirelessEnabled": dbus.Variant{false}},
+	)
+	nm := New(tc, s.log)
+	ch, w, err := nm.WatchWirelessEnabled()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	v, ok := <-ch
+	c.Check(ok, Equals, true)
+	c.Check(v, Equals, false)
+}

=== added directory 'bus/urfkill'
=== added file 'bus/urfkill/urfkill.go'
--- bus/urfkill/urfkill.go	1970-01-01 00:00:00 +0000
+++ bus/urfkill/urfkill.go	2015-04-02 09:52:21 +0000
@@ -0,0 +1,94 @@
+/*
+ Copyright 2015 Canonical Ltd.
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+// Package urfkill wraps a couple of URfkill's DBus API points to
+// watch for flight mode state changes.
+package urfkill
+
+import (
+	//"launchpad.net/go-dbus/v1"
+
+	"launchpad.net/ubuntu-push/bus"
+	"launchpad.net/ubuntu-push/logger"
+)
+
+// URfkill lives on a well-knwon bus.Address
+var BusAddress bus.Address = bus.Address{
+	Interface: "org.freedesktop.URfkill",
+	Path:      "/org/freedesktop/URfkill",
+	Name:      "org.freedesktop.URfkill",
+}
+
+/*****************************************************************
+ *    URfkill (and its implementation)
+ */
+
+type URfkill interface {
+	// IsFlightMode returns flight mode state.
+	IsFlightMode() bool
+	// WatchFlightMode listens for changes to URfkill's flight
+	// mode state, and sends them out over the channel returned.
+	WatchFlightMode() (<-chan bool, bus.Cancellable, error)
+}
+
+type uRfkill struct {
+	bus bus.Endpoint
+	log logger.Logger
+}
+
+// New returns a new URfkill that'll use the provided bus.Endpoint
+func New(endp bus.Endpoint, log logger.Logger) URfkill {
+	return &uRfkill{endp, log}
+}
+
+// ensure uRfkill implements URfkill
+var _ URfkill = &uRfkill{}
+
+/*
+   public methods
+*/
+
+func (ur *uRfkill) IsFlightMode() bool {
+	var res bool
+	err := ur.bus.Call("IsFlightMode", bus.Args(), &res)
+	if err != nil {
+		ur.log.Errorf("failed getting flight-mode state: %s", err)
+		ur.log.Debugf("defaulting flight-mode state to false")
+		return false
+	}
+	return res
+}
+
+func (ur *uRfkill) WatchFlightMode() (<-chan bool, bus.Cancellable, error) {
+	ch := make(chan bool)
+	w, err := ur.bus.WatchSignal("FlightModeChanged",
+		func(ns ...interface{}) {
+			stbool, ok := ns[0].(bool)
+			if !ok {
+				ur.log.Errorf("got weird flight-mode state: %#v", ns[0])
+				return
+			}
+			ur.log.Debugf("got flight-mode change: %v", stbool)
+			ch <- stbool
+		},
+		func() { close(ch) })
+	if err != nil {
+		ur.log.Debugf("Failed to set up the watch: %s", err)
+		return nil, nil, err
+	}
+
+	return ch, w, nil
+}

=== added file 'bus/urfkill/urfkill.go_test.go'
--- bus/urfkill/urfkill.go_test.go	1970-01-01 00:00:00 +0000
+++ bus/urfkill/urfkill.go_test.go	2015-04-02 09:52:21 +0000
@@ -0,0 +1,121 @@
+/*
+ Copyright 2015 Canonical Ltd.
+
+ This program is free software: you can redistribute it and/or modify it
+ under the terms of the GNU General Public License version 3, as published
+ by the Free Software Foundation.
+
+ This program is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranties of
+ MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ PURPOSE.  See the GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+package urfkill
+
+import (
+	"testing"
+
+	//"launchpad.net/go-dbus/v1"
+	. "launchpad.net/gocheck"
+
+	testingbus "launchpad.net/ubuntu-push/bus/testing"
+	"launchpad.net/ubuntu-push/logger"
+	helpers "launchpad.net/ubuntu-push/testing"
+	"launchpad.net/ubuntu-push/testing/condition"
+)
+
+// hook up gocheck
+func Test(t *testing.T) { TestingT(t) }
+
+type URSuite struct {
+	log logger.Logger
+}
+
+var _ = Suite(&URSuite{})
+
+func (s *URSuite) SetUpTest(c *C) {
+	s.log = helpers.NewTestLogger(c, "debug")
+}
+
+func (s *URSuite) TestNew(c *C) {
+	ur := New(testingbus.NewTestingEndpoint(nil, condition.Work(true)), s.log)
+	c.Check(ur, NotNil)
+}
+
+// IsFlightMode returns the right state when everything works
+func (s *URSuite) TestIsFlightMode(c *C) {
+	endp := testingbus.NewTestingEndpoint(nil, condition.Work(true), true)
+	ur := New(endp, s.log)
+	state := ur.IsFlightMode()
+	c.Check(state, Equals, true)
+	callArgs := testingbus.GetCallArgs(endp)
+	c.Assert(callArgs, HasLen, 1)
+	c.Assert(callArgs[0].Member, Equals, "IsFlightMode")
+	c.Assert(callArgs[0].Args, HasLen, 0)
+}
+
+// IsFlightMode returns the right state when dbus fails
+func (s *URSuite) TestIsFlightModeFail(c *C) {
+	ur := New(testingbus.NewTestingEndpoint(nil, condition.Work(false)), s.log)
+	state := ur.IsFlightMode()
+	c.Check(state, Equals, false)
+}
+
+// IsFlightMode returns the right state when dbus works but delivers
+// rubbish values
+func (s *URSuite) TestIsFlightModeRubbishValues(c *C) {
+	ur := New(testingbus.NewTestingEndpoint(nil, condition.Work(true), "broken"), s.log)
+	state := ur.IsFlightMode()
+	c.Check(state, Equals, false)
+}
+
+// IsFlightMode returns the right state when dbus works but delivers a rubbish structure
+func (s *URSuite) TestIsFlightModeRubbishStructure(c *C) {
+	ur := New(testingbus.NewMultiValuedTestingEndpoint(nil, condition.Work(true), []interface{}{}), s.log)
+	state := ur.IsFlightMode()
+	c.Check(state, Equals, false)
+}
+
+// WatchFightMode sends a stream of states over the channel
+func (s *URSuite) TestWatchFlightMode(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true), false, true, false)
+	ur := New(tc, s.log)
+	ch, w, err := ur.WatchFlightMode()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	l := []bool{<-ch, <-ch, <-ch}
+	c.Check(l, DeepEquals, []bool{false, true, false})
+}
+
+// WatchFlightMode returns on error if the dbus call fails
+func (s *URSuite) TestWatchFlightModeFails(c *C) {
+	ur := New(testingbus.NewTestingEndpoint(nil, condition.Work(false)), s.log)
+	_, _, err := ur.WatchFlightMode()
+	c.Check(err, NotNil)
+}
+
+// WatchFlightMode calls close on its channel when the watch bails
+func (s *URSuite) TestWatchFlightModeClosesOnWatchBail(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true))
+	ur := New(tc, s.log)
+	ch, w, err := ur.WatchFlightMode()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	_, ok := <-ch
+	c.Check(ok, Equals, false)
+}
+
+// WatchFlightMode survives rubbish values
+func (s *URSuite) TestWatchFlightModeSurvivesRubbishValues(c *C) {
+	tc := testingbus.NewTestingEndpoint(nil, condition.Work(true), "gorp")
+	ur := New(tc, s.log)
+	ch, w, err := ur.WatchFlightMode()
+	c.Assert(err, IsNil)
+	defer w.Cancel()
+	_, ok := <-ch
+	c.Check(ok, Equals, false)
+}

=== modified file 'client/client.go'
--- client/client.go	2015-02-27 10:47:17 +0000
+++ client/client.go	2015-04-02 09:52:21 +0000
@@ -79,6 +79,7 @@
 	PollNetworkWait config.ConfigTimeDuration `json:"poll_net_wait"`
 	PollPolldWait   config.ConfigTimeDuration `json:"poll_polld_wait"`
 	PollDoneWait    config.ConfigTimeDuration `json:"poll_done_wait"`
+	PollBusyWait    config.ConfigTimeDuration `json:"poll_busy_wait"`
 }
 
 // PushService is the interface we use of service.PushService.
@@ -115,8 +116,7 @@
 	systemImageEndp    bus.Endpoint
 	systemImageInfo    *systemimage.InfoResult
 	connCh             chan bool
-	hasConnectivity    bool
-	session            *session.ClientSession
+	session            session.ClientSession
 	sessionConnectedCh chan uint32
 	pushService        PushService
 	postalService      PostalService
@@ -125,16 +125,20 @@
 	installedChecker   click.InstalledChecker
 	poller             poller.Poller
 	accountsCh         <-chan accounts.Changed
+	// session-side channels
+	broadcastCh     chan *session.BroadcastNotification
+	notificationsCh chan session.AddressedNotification
 }
 
 // Creates a new Ubuntu Push Notifications client-side daemon that will use
 // the given configuration file.
 func NewPushClient(configPath string, leveldbPath string) *PushClient {
-	client := new(PushClient)
-	client.configPath = configPath
-	client.leveldbPath = leveldbPath
-
-	return client
+	return &PushClient{
+		configPath:      configPath,
+		leveldbPath:     leveldbPath,
+		broadcastCh:     make(chan *session.BroadcastNotification),
+		notificationsCh: make(chan session.AddressedNotification),
+	}
 }
 
 var newIdentifier = identifier.New
@@ -206,6 +210,8 @@
 		AuthGetter:       client.getAuthorization,
 		AuthURL:          client.config.SessionURL,
 		AddresseeChecker: client,
+		BroadcastCh:      client.broadcastCh,
+		NotificationsCh:  client.notificationsCh,
 	}
 }
 
@@ -241,6 +247,7 @@
 			NetworkWait:        client.config.PollNetworkWait.TimeDuration(),
 			PolldWait:          client.config.PollPolldWait.TimeDuration(),
 			DoneWait:           client.config.PollDoneWait.TimeDuration(),
+			BusyWait:           client.config.PollBusyWait.TimeDuration(),
 		},
 		Log:                client.log,
 		SessionStateGetter: client.session,
@@ -280,7 +287,6 @@
 
 // takeTheBus starts the connection(s) to D-Bus and sets up associated event channels
 func (client *PushClient) takeTheBus() error {
-	fmt.Println("FOO")
 	cs := connectivity.New(client.connectivityEndp,
 		client.config.ConnectivityConfig, client.log)
 	go cs.Track(client.connCh)
@@ -308,6 +314,7 @@
 		return err
 	}
 	client.session = sess
+	sess.KeepConnection()
 	client.poller = poller.New(client.derivePollerSetup())
 	return nil
 }
@@ -376,29 +383,6 @@
 	}
 }
 
-// handleConnState deals with connectivity events
-func (client *PushClient) handleConnState(hasConnectivity bool) {
-	client.log.Debugf("handleConnState: %v", hasConnectivity)
-	if client.hasConnectivity == hasConnectivity {
-		// nothing to do!
-		return
-	}
-	client.hasConnectivity = hasConnectivity
-	client.session.Close()
-	if hasConnectivity {
-		client.session.AutoRedial(client.sessionConnectedCh)
-	}
-}
-
-// handleErr deals with the session erroring out of its loop
-func (client *PushClient) handleErr(err error) {
-	// if we're not connected, we don't really care
-	client.log.Errorf("session exited: %s", err)
-	if client.hasConnectivity {
-		client.session.AutoRedial(client.sessionConnectedCh)
-	}
-}
-
 // filterBroadcastNotification finds out if the notification is about an actual
 // upgrade for the device. It expects msg.Decoded entries to look
 // like:
@@ -426,20 +410,9 @@
 	if len(pair) < 1 {
 		return false
 	}
-	buildNumber, ok := pair[0].(float64)
-	if !ok {
-		return false
-	}
-	curBuildNumber := float64(client.systemImageInfo.BuildNumber)
-	if buildNumber > curBuildNumber {
-		return true
-	}
-	// xxx we should really compare channel_target and alias here
-	// going backward by a margin, assume switch of target
-	if buildNumber < curBuildNumber && (curBuildNumber-buildNumber) > 10 {
-		return true
-	}
-	return false
+	_, ok = pair[0].(float64)
+	// ok means it sanity checks, let the helper check for build number etc
+	return ok
 }
 
 // handleBroadcastNotification deals with receiving a broadcast notification
@@ -469,28 +442,18 @@
 	return nil
 }
 
-// handleAccountsChange deals with the user adding or removing (or
-// changing) the u1 account used to auth
-func (client *PushClient) handleAccountsChange() {
-	client.log.Infof("U1 account changed; restarting session")
-	client.session.ClearCookie()
-	client.session.Close()
-}
-
 // doLoop connects events with their handlers
-func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, errhandler func(error), unregisterhandler func(*click.AppId), accountshandler func()) {
+func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, unregisterhandler func(*click.AppId), accountshandler func()) {
 	for {
 		select {
 		case <-client.accountsCh:
 			accountshandler()
 		case state := <-client.connCh:
 			connhandler(state)
-		case bcast := <-client.session.BroadcastCh:
+		case bcast := <-client.broadcastCh:
 			bcasthandler(bcast)
-		case aucast := <-client.session.NotificationsCh:
+		case aucast := <-client.notificationsCh:
 			ucasthandler(aucast)
-		case err := <-client.session.ErrCh:
-			errhandler(err)
 		case count := <-client.sessionConnectedCh:
 			client.log.Debugf("session connected after %d attempts", count)
 		case app := <-client.unregisterCh:
@@ -512,12 +475,11 @@
 
 // Loop calls doLoop with the "real" handlers
 func (client *PushClient) Loop() {
-	client.doLoop(client.handleConnState,
+	client.doLoop(client.session.HasConnectivity,
 		client.handleBroadcastNotification,
 		client.handleUnicastNotification,
-		client.handleErr,
 		client.handleUnregister,
-		client.handleAccountsChange,
+		client.session.ResetCookie,
 	)
 }
 

=== modified file 'client/client_test.go'
--- client/client_test.go	2015-02-27 10:47:17 +0000
+++ client/client_test.go	2015-04-02 09:52:21 +0000
@@ -43,7 +43,6 @@
 	clickhelp "launchpad.net/ubuntu-push/click/testing"
 	"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/identifier"
 	idtesting "launchpad.net/ubuntu-push/identifier/testing"
@@ -186,6 +185,7 @@
 		"poll_net_wait":    "1m",
 		"poll_polld_wait":  "3m",
 		"poll_done_wait":   "5s",
+		"poll_busy_wait":   "0s",
 	}
 	for k, v := range overrides {
 		cfgMap[k] = v
@@ -427,6 +427,8 @@
 		AuthGetter:       func(string) string { return "" },
 		AuthURL:          "xyzzy://",
 		AddresseeChecker: cli,
+		BroadcastCh:      make(chan *session.BroadcastNotification),
+		NotificationsCh:  make(chan session.AddressedNotification),
 	}
 	// sanity check that we are looking at all fields
 	vExpected := reflect.ValueOf(expected)
@@ -440,6 +442,11 @@
 	conf := cli.deriveSessionConfig(info)
 	// compare authGetter by string
 	c.Check(fmt.Sprintf("%#v", conf.AuthGetter), Equals, fmt.Sprintf("%#v", cli.getAuthorization))
+	// channels are ok as long as non-nil
+	conf.BroadcastCh = nil
+	conf.NotificationsCh = nil
+	expected.BroadcastCh = nil
+	expected.NotificationsCh = nil
 	// and set it to nil
 	conf.AuthGetter = nil
 	expected.AuthGetter = nil
@@ -521,10 +528,18 @@
 /*****************************************************************
     derivePollerSetup tests
 ******************************************************************/
+type derivePollerSession struct{}
+
+func (s *derivePollerSession) ResetCookie()                      {}
+func (s *derivePollerSession) State() session.ClientSessionState { return session.Unknown }
+func (s *derivePollerSession) HasConnectivity(bool)              {}
+func (s *derivePollerSession) KeepConnection() error             { return nil }
+func (s *derivePollerSession) StopKeepConnection()               {}
+
 func (cs *clientSuite) TestDerivePollerSetup(c *C) {
 	cs.writeTestConfig(map[string]interface{}{})
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.session = new(session.ClientSession)
+	cli.session = new(derivePollerSession)
 	err := cli.configure()
 	c.Assert(err, IsNil)
 	expected := &poller.PollerSetup{
@@ -705,22 +720,6 @@
 }
 
 /*****************************************************************
-    handleErr tests
-******************************************************************/
-
-func (cs *clientSuite) TestHandleErr(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	cli.systemImageInfo = siInfoRes
-	c.Assert(cli.initSessionAndPoller(), IsNil)
-	cs.log.ResetCapture()
-	cli.hasConnectivity = true
-	defer cli.session.Close()
-	cli.handleErr(errors.New("bananas"))
-	c.Check(cs.log.Captured(), Matches, ".*session exited.*bananas\n")
-}
-
-/*****************************************************************
     seenStateFactory tests
 ******************************************************************/
 
@@ -741,60 +740,6 @@
 }
 
 /*****************************************************************
-    handleConnState tests
-******************************************************************/
-
-func (cs *clientSuite) TestHandleConnStateD2C(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	cli.systemImageInfo = siInfoRes
-	c.Assert(cli.initSessionAndPoller(), IsNil)
-
-	c.Assert(cli.hasConnectivity, Equals, false)
-	defer cli.session.Close()
-	cli.handleConnState(true)
-	c.Check(cli.hasConnectivity, Equals, true)
-	c.Assert(cli.session, NotNil)
-}
-
-func (cs *clientSuite) TestHandleConnStateSame(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	// here we want to check that we don't do anything
-	c.Assert(cli.session, IsNil)
-	c.Assert(cli.hasConnectivity, Equals, false)
-	cli.handleConnState(false)
-	c.Check(cli.session, IsNil)
-
-	cli.hasConnectivity = true
-	cli.handleConnState(true)
-	c.Check(cli.session, IsNil)
-}
-
-func (cs *clientSuite) TestHandleConnStateC2D(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	cli.session, _ = session.NewSession(cli.config.Addr, cli.deriveSessionConfig(nil), cli.deviceId, seenstate.NewSeenState, cs.log)
-	cli.session.Dial()
-	cli.hasConnectivity = true
-
-	// cli.session.State() will be "Error" here, for now at least
-	c.Check(cli.session.State(), Not(Equals), session.Disconnected)
-	cli.handleConnState(false)
-	c.Check(cli.session.State(), Equals, session.Disconnected)
-}
-
-func (cs *clientSuite) TestHandleConnStateC2DPending(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	cli.session, _ = session.NewSession(cli.config.Addr, cli.deriveSessionConfig(nil), cli.deviceId, seenstate.NewSeenState, cs.log)
-	cli.hasConnectivity = true
-
-	cli.handleConnState(false)
-	c.Check(cli.session.State(), Equals, session.Disconnected)
-}
-
-/*****************************************************************
    filterBroadcastNotification tests
 ******************************************************************/
 
@@ -811,35 +756,11 @@
 	// empty
 	msg := &session.BroadcastNotification{}
 	c.Check(cli.filterBroadcastNotification(msg), Equals, false)
-	// same build number
-	msg = &session.BroadcastNotification{
-		Decoded: []map[string]interface{}{
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(102), "tubular"},
-			},
-		},
-	}
-	c.Check(cli.filterBroadcastNotification(msg), Equals, false)
-	// higher build number and pick last
-	msg = &session.BroadcastNotification{
-		Decoded: []map[string]interface{}{
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(102), "tubular"},
-			},
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(103), "tubular"},
-			},
-		},
-	}
-	c.Check(cli.filterBroadcastNotification(msg), Equals, true)
-	// going backward by a margin, assume switch of alias
-	msg = &session.BroadcastNotification{
-		Decoded: []map[string]interface{}{
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(102), "tubular"},
-			},
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(2), "urban"},
+	// same build number, we let the helper deal
+	msg = &session.BroadcastNotification{
+		Decoded: []map[string]interface{}{
+			map[string]interface{}{
+				"daily/mako": []interface{}{float64(102), "tubular"},
 			},
 		},
 	}
@@ -888,9 +809,7 @@
 	}
 	negativeBroadcastNotification = &session.BroadcastNotification{
 		Decoded: []map[string]interface{}{
-			map[string]interface{}{
-				"daily/mako": []interface{}{float64(102), "tubular"},
-			},
+			map[string]interface{}{},
 		},
 	}
 )
@@ -1012,7 +931,6 @@
 var nopConn = func(bool) {}
 var nopBcast = func(*session.BroadcastNotification) error { return nil }
 var nopUcast = func(session.AddressedNotification) error { return nil }
-var nopError = func(error) {}
 var nopUnregister = func(*click.AppId) {}
 var nopAcct = func() {}
 
@@ -1025,7 +943,7 @@
 	c.Assert(cli.initSessionAndPoller(), IsNil)
 
 	ch := make(chan bool, 1)
-	go cli.doLoop(func(bool) { ch <- true }, nopBcast, nopUcast, nopError, nopUnregister, nopAcct)
+	go cli.doLoop(func(bool) { ch <- true }, nopBcast, nopUcast, nopUnregister, nopAcct)
 	c.Check(takeNextBool(ch), Equals, true)
 }
 
@@ -1034,11 +952,11 @@
 	cli.log = cs.log
 	cli.systemImageInfo = siInfoRes
 	c.Assert(cli.initSessionAndPoller(), IsNil)
-	cli.session.BroadcastCh = make(chan *session.BroadcastNotification, 1)
-	cli.session.BroadcastCh <- &session.BroadcastNotification{}
+	cli.broadcastCh = make(chan *session.BroadcastNotification, 1)
+	cli.broadcastCh <- &session.BroadcastNotification{}
 
 	ch := make(chan bool, 1)
-	go cli.doLoop(nopConn, func(_ *session.BroadcastNotification) error { ch <- true; return nil }, nopUcast, nopError, nopUnregister, nopAcct)
+	go cli.doLoop(nopConn, func(_ *session.BroadcastNotification) error { ch <- true; return nil }, nopUcast, nopUnregister, nopAcct)
 	c.Check(takeNextBool(ch), Equals, true)
 }
 
@@ -1047,24 +965,11 @@
 	cli.log = cs.log
 	cli.systemImageInfo = siInfoRes
 	c.Assert(cli.initSessionAndPoller(), IsNil)
-	cli.session.NotificationsCh = make(chan session.AddressedNotification, 1)
-	cli.session.NotificationsCh <- session.AddressedNotification{}
-
-	ch := make(chan bool, 1)
-	go cli.doLoop(nopConn, nopBcast, func(session.AddressedNotification) error { ch <- true; return nil }, nopError, nopUnregister, nopAcct)
-	c.Check(takeNextBool(ch), Equals, true)
-}
-
-func (cs *clientSuite) TestDoLoopErr(c *C) {
-	cli := NewPushClient(cs.configPath, cs.leveldbPath)
-	cli.log = cs.log
-	cli.systemImageInfo = siInfoRes
-	c.Assert(cli.initSessionAndPoller(), IsNil)
-	cli.session.ErrCh = make(chan error, 1)
-	cli.session.ErrCh <- nil
-
-	ch := make(chan bool, 1)
-	go cli.doLoop(nopConn, nopBcast, nopUcast, func(error) { ch <- true }, nopUnregister, nopAcct)
+	cli.notificationsCh = make(chan session.AddressedNotification, 1)
+	cli.notificationsCh <- session.AddressedNotification{}
+
+	ch := make(chan bool, 1)
+	go cli.doLoop(nopConn, nopBcast, func(session.AddressedNotification) error { ch <- true; return nil }, nopUnregister, nopAcct)
 	c.Check(takeNextBool(ch), Equals, true)
 }
 
@@ -1077,7 +982,7 @@
 	cli.unregisterCh <- app1
 
 	ch := make(chan bool, 1)
-	go cli.doLoop(nopConn, nopBcast, nopUcast, nopError, func(app *click.AppId) { c.Check(app.Original(), Equals, appId1); ch <- true }, nopAcct)
+	go cli.doLoop(nopConn, nopBcast, nopUcast, func(app *click.AppId) { c.Check(app.Original(), Equals, appId1); ch <- true }, nopAcct)
 	c.Check(takeNextBool(ch), Equals, true)
 }
 
@@ -1091,7 +996,7 @@
 	cli.accountsCh = acctCh
 
 	ch := make(chan bool, 1)
-	go cli.doLoop(nopConn, nopBcast, nopUcast, nopError, nopUnregister, func() { ch <- true })
+	go cli.doLoop(nopConn, nopBcast, nopUcast, nopUnregister, func() { ch <- true })
 	c.Check(takeNextBool(ch), Equals, true)
 }
 
@@ -1126,6 +1031,20 @@
     Loop() tests
 ******************************************************************/
 
+type loopSession struct{ hasConn bool }
+
+func (s *loopSession) ResetCookie() {}
+func (s *loopSession) State() session.ClientSessionState {
+	if s.hasConn {
+		return session.Connected
+	} else {
+		return session.Disconnected
+	}
+}
+func (s *loopSession) HasConnectivity(hasConn bool) { s.hasConn = hasConn }
+func (s *loopSession) KeepConnection() error        { return nil }
+func (s *loopSession) StopKeepConnection()          {}
+
 func (cs *clientSuite) TestLoop(c *C) {
 	cli := NewPushClient(cs.configPath, cs.leveldbPath)
 	cli.connCh = make(chan bool)
@@ -1140,8 +1059,7 @@
 
 	c.Assert(cli.initSessionAndPoller(), IsNil)
 
-	cli.session.BroadcastCh = make(chan *session.BroadcastNotification)
-	cli.session.ErrCh = make(chan error)
+	cli.broadcastCh = make(chan *session.BroadcastNotification)
 
 	// we use tick() to make sure things have been through the
 	// event loop at least once before looking at things;
@@ -1149,6 +1067,10 @@
 	// at and the loop itself.
 	tick := func() { cli.sessionConnectedCh <- 42 }
 
+	c.Assert(cli.session, NotNil)
+	cli.session.StopKeepConnection()
+	cli.session = &loopSession{}
+
 	go cli.Loop()
 
 	// sessionConnectedCh to nothing in particular, but it'll help sync this test
@@ -1158,24 +1080,19 @@
 
 	// loop() should have connected:
 	//  * connCh to the connectivity checker
-	c.Check(cli.hasConnectivity, Equals, false)
+	c.Check(cli.session.State(), Equals, session.Disconnected)
 	cli.connCh <- true
 	tick()
-	c.Check(cli.hasConnectivity, Equals, true)
+	c.Check(cli.session.State(), Equals, session.Connected)
 	cli.connCh <- false
 	tick()
-	c.Check(cli.hasConnectivity, Equals, false)
+	c.Check(cli.session.State(), Equals, session.Disconnected)
 
 	//  * session.BroadcastCh to the notifications handler
 	c.Check(d.bcastCount, Equals, 0)
-	cli.session.BroadcastCh <- positiveBroadcastNotification
+	cli.broadcastCh <- positiveBroadcastNotification
 	tick()
 	c.Check(d.bcastCount, Equals, 1)
-
-	//  * session.ErrCh to the error handler
-	cli.session.ErrCh <- nil
-	tick()
-	c.Check(cs.log.Captured(), Matches, "(?ms).*session exited.*")
 }
 
 /*****************************************************************

=== modified file 'client/session/session.go'
--- client/session/session.go	2015-01-22 11:05:37 +0000
+++ client/session/session.go	2015-04-02 09:52:21 +0000
@@ -39,6 +39,14 @@
 	"launchpad.net/ubuntu-push/util"
 )
 
+type sessCmd uint8
+
+const (
+	cmdDisconnect sessCmd = iota
+	cmdConnect
+	cmdResetCookie
+)
+
 var (
 	wireVersionBytes = []byte{protocol.ProtocolWireVersion}
 )
@@ -70,10 +78,12 @@
 
 const (
 	Error ClientSessionState = iota
+	Pristine
 	Disconnected
 	Connected
 	Started
 	Running
+	Shutdown
 	Unknown
 )
 
@@ -83,10 +93,12 @@
 	}
 	return [Unknown]string{
 		"Error",
+		"Pristine",
 		"Disconnected",
 		"Connected",
 		"Started",
 		"Running",
+		"Shutdown",
 	}[s]
 }
 
@@ -118,10 +130,20 @@
 	AuthGetter             func(string) string
 	AuthURL                string
 	AddresseeChecker       AddresseeChecking
+	BroadcastCh            chan *BroadcastNotification
+	NotificationsCh        chan AddressedNotification
 }
 
 // ClientSession holds a client<->server session and its configuration.
-type ClientSession struct {
+type ClientSession interface {
+	ResetCookie()
+	State() ClientSessionState
+	HasConnectivity(bool)
+	KeepConnection() error
+	StopKeepConnection()
+}
+
+type clientSession struct {
 	// configuration
 	DeviceId string
 	ClientSessionConfig
@@ -145,25 +167,36 @@
 	proto        protocol.Protocol
 	pingInterval time.Duration
 	retrier      util.AutoRedialer
-	retrierLock  sync.Mutex
 	cookie       string
 	// status
-	stateP          *uint32
-	ErrCh           chan error
-	BroadcastCh     chan *BroadcastNotification
-	NotificationsCh chan AddressedNotification
+	stateLock sync.RWMutex
+	state     ClientSessionState
 	// authorization
 	auth string
 	// autoredial knobs
 	shouldDelayP    *uint32
 	lastAutoRedial  time.Time
-	redialDelay     func(*ClientSession) time.Duration
+	redialDelay     func(*clientSession) time.Duration
 	redialJitter    func(time.Duration) time.Duration
 	redialDelays    []time.Duration
 	redialDelaysIdx int
+	// connection events, and cookie reset requests, come in over here
+	cmdCh chan sessCmd
+	// last seen connection event is here
+	lastConn bool
+	// connection events are handled by this
+	connHandler func(bool)
+	// autoredial goes over here (xxx spurious goroutine involved)
+	doneCh chan uint32
+	// main loop errors out through here (possibly another spurious goroutine)
+	errCh chan error
+	// main loop errors are handled by this
+	errHandler func(error)
+	// look, a stopper!
+	stopCh chan struct{}
 }
 
-func redialDelay(sess *ClientSession) time.Duration {
+func redialDelay(sess *clientSession) time.Duration {
 	if sess.ShouldDelay() {
 		t := sess.redialDelays[sess.redialDelaysIdx]
 		if len(sess.redialDelays) > sess.redialDelaysIdx+1 {
@@ -178,8 +211,7 @@
 
 func NewSession(serverAddrSpec string, conf ClientSessionConfig,
 	deviceId string, seenStateFactory func() (seenstate.SeenState, error),
-	log logger.Logger) (*ClientSession, error) {
-	state := uint32(Disconnected)
+	log logger.Logger) (*clientSession, error) {
 	seenState, err := seenStateFactory()
 	if err != nil {
 		return nil, err
@@ -191,7 +223,7 @@
 		getHost = gethosts.New(deviceId, hostsEndpoint, conf.ExchangeTimeout)
 	}
 	var shouldDelay uint32 = 0
-	sess := &ClientSession{
+	sess := &clientSession{
 		ClientSessionConfig: conf,
 		getHost:             getHost,
 		fallbackHosts:       fallbackHosts,
@@ -200,10 +232,10 @@
 		Protocolator:        protocol.NewProtocol0,
 		SeenState:           seenState,
 		TLS:                 &tls.Config{},
-		stateP:              &state,
+		state:               Pristine,
 		timeSince:           time.Since,
 		shouldDelayP:        &shouldDelay,
-		redialDelay:         redialDelay,
+		redialDelay:         redialDelay, // NOTE there are tests that use calling sess.redialDelay as an indication of calling autoRedial!
 		redialDelays:        util.Timeouts(),
 	}
 	sess.redialJitter = sess.Jitter
@@ -215,62 +247,78 @@
 		}
 		sess.TLS.RootCAs = cp
 	}
+	sess.doneCh = make(chan uint32, 1)
+	sess.stopCh = make(chan struct{})
+	sess.cmdCh = make(chan sessCmd)
+	sess.errCh = make(chan error, 1)
+
+	// to be overridden by tests
+	sess.connHandler = sess.handleConn
+	sess.errHandler = sess.handleErr
+
 	return sess, nil
 }
 
-func (sess *ClientSession) ShouldDelay() bool {
+func (sess *clientSession) ShouldDelay() bool {
 	return atomic.LoadUint32(sess.shouldDelayP) != 0
 }
 
-func (sess *ClientSession) setShouldDelay() {
+func (sess *clientSession) setShouldDelay() {
 	atomic.StoreUint32(sess.shouldDelayP, uint32(1))
 }
 
-func (sess *ClientSession) clearShouldDelay() {
+func (sess *clientSession) clearShouldDelay() {
 	atomic.StoreUint32(sess.shouldDelayP, uint32(0))
 }
 
-func (sess *ClientSession) State() ClientSessionState {
-	return ClientSessionState(atomic.LoadUint32(sess.stateP))
-}
-
-func (sess *ClientSession) setState(state ClientSessionState) {
-	sess.Log.Debugf("session.setState: %s -> %s", ClientSessionState(atomic.LoadUint32(sess.stateP)), state)
-	atomic.StoreUint32(sess.stateP, uint32(state))
-}
-
-func (sess *ClientSession) setConnection(conn net.Conn) {
+func (sess *clientSession) State() ClientSessionState {
+	sess.stateLock.RLock()
+	defer sess.stateLock.RUnlock()
+	return sess.state
+}
+
+func (sess *clientSession) setState(state ClientSessionState) {
+	sess.stateLock.Lock()
+	defer sess.stateLock.Unlock()
+	sess.Log.Debugf("session.setState: %s -> %s", sess.state, state)
+	sess.state = state
+}
+
+func (sess *clientSession) setConnection(conn net.Conn) {
 	sess.connLock.Lock()
 	defer sess.connLock.Unlock()
 	sess.Connection = conn
 }
 
-func (sess *ClientSession) getConnection() net.Conn {
+func (sess *clientSession) getConnection() net.Conn {
 	sess.connLock.RLock()
 	defer sess.connLock.RUnlock()
 	return sess.Connection
 }
 
-func (sess *ClientSession) setCookie(cookie string) {
+func (sess *clientSession) setCookie(cookie string) {
 	sess.connLock.Lock()
 	defer sess.connLock.Unlock()
 	sess.cookie = cookie
 }
 
-func (sess *ClientSession) getCookie() string {
+func (sess *clientSession) getCookie() string {
 	sess.connLock.RLock()
 	defer sess.connLock.RUnlock()
 	return sess.cookie
 }
 
-func (sess *ClientSession) ClearCookie() {
-	sess.connLock.Lock()
-	defer sess.connLock.Unlock()
-	sess.cookie = ""
+func (sess *clientSession) ResetCookie() {
+	sess.cmdCh <- cmdResetCookie
+}
+
+func (sess *clientSession) resetCookie() {
+	sess.stopRedial()
+	sess.doClose(true)
 }
 
 // getHosts sets deliveryHosts possibly querying a remote endpoint
-func (sess *ClientSession) getHosts() error {
+func (sess *clientSession) getHosts() error {
 	if sess.getHost != nil {
 		if sess.deliveryHosts != nil && sess.timeSince(sess.deliveryHostsTimestamp) < sess.HostsCachingExpiryTime {
 			return nil
@@ -294,7 +342,7 @@
 
 // addAuthorization gets the authorization blob to send to the server
 // and adds it to the session.
-func (sess *ClientSession) addAuthorization() error {
+func (sess *clientSession) addAuthorization() error {
 	if sess.AuthGetter != nil {
 		sess.Log.Debugf("adding authorization")
 		sess.auth = sess.AuthGetter(sess.AuthURL)
@@ -302,13 +350,13 @@
 	return nil
 }
 
-func (sess *ClientSession) resetHosts() {
+func (sess *clientSession) resetHosts() {
 	sess.deliveryHosts = nil
 }
 
 // startConnectionAttempt/nextHostToTry help connect iterating over candidate hosts
 
-func (sess *ClientSession) startConnectionAttempt() {
+func (sess *clientSession) startConnectionAttempt() {
 	if sess.timeSince(sess.lastAttemptTimestamp) > sess.ExpectAllRepairedTime {
 		sess.tryHost = 0
 	}
@@ -319,7 +367,7 @@
 	sess.lastAttemptTimestamp = time.Now()
 }
 
-func (sess *ClientSession) nextHostToTry() string {
+func (sess *clientSession) nextHostToTry() string {
 	if sess.leftToTry == 0 {
 		return ""
 	}
@@ -331,7 +379,7 @@
 
 // we reached the Started state, we can retry with the same host if we
 // have to retry again
-func (sess *ClientSession) started() {
+func (sess *clientSession) started() {
 	sess.tryHost--
 	if sess.tryHost == -1 {
 		sess.tryHost = len(sess.deliveryHosts) - 1
@@ -341,7 +389,7 @@
 
 // connect to a server using the configuration in the ClientSession
 // and set up the connection.
-func (sess *ClientSession) connect() error {
+func (sess *clientSession) connect() error {
 	sess.setShouldDelay()
 	sess.startConnectionAttempt()
 	var err error
@@ -363,49 +411,47 @@
 	return nil
 }
 
-func (sess *ClientSession) stopRedial() {
-	sess.retrierLock.Lock()
-	defer sess.retrierLock.Unlock()
+func (sess *clientSession) stopRedial() {
 	if sess.retrier != nil {
 		sess.retrier.Stop()
 		sess.retrier = nil
 	}
 }
 
-func (sess *ClientSession) AutoRedial(doneCh chan uint32) {
+func (sess *clientSession) autoRedial() {
 	sess.stopRedial()
 	if time.Since(sess.lastAutoRedial) < 2*time.Second {
 		sess.setShouldDelay()
 	}
-	time.Sleep(sess.redialDelay(sess))
-	sess.retrierLock.Lock()
-	defer sess.retrierLock.Unlock()
+	// xxx should we really wait on the caller goroutine?
+	delay := sess.redialDelay(sess)
+	sess.Log.Debugf("session redial delay: %v, wait", delay)
+	time.Sleep(delay)
+	sess.Log.Debugf("session redial delay: %v, cont", delay)
 	if sess.retrier != nil {
 		panic("session AutoRedial: unexpected non-nil retrier.")
 	}
 	sess.retrier = util.NewAutoRedialer(sess)
 	sess.lastAutoRedial = time.Now()
-	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
-		}
+	go func(retrier util.AutoRedialer) {
 		sess.Log.Debugf("session autoredialier launching Redial goroutine")
-		doneCh <- retrier.Redial()
-	}()
-}
-
-func (sess *ClientSession) Close() {
-	sess.stopRedial()
-	sess.doClose()
-}
-
-func (sess *ClientSession) doClose() {
+		// if the redialer has been stopped before calling Redial(), it'll return 0.
+		sess.doneCh <- retrier.Redial()
+	}(sess.retrier)
+}
+
+func (sess *clientSession) doClose(resetCookie bool) {
 	sess.connLock.Lock()
 	defer sess.connLock.Unlock()
+	if resetCookie {
+		sess.cookie = ""
+	}
+	sess.closeConnection()
+	sess.setState(Disconnected)
+}
+
+func (sess *clientSession) closeConnection() {
+	// *must be called with connLock held*
 	if sess.Connection != nil {
 		sess.Connection.Close()
 		// we ignore Close errors, on purpose (the thinking being that
@@ -413,11 +459,10 @@
 		// you could do to recover at this stage).
 		sess.Connection = nil
 	}
-	sess.setState(Disconnected)
 }
 
 // handle "ping" messages
-func (sess *ClientSession) handlePing() error {
+func (sess *clientSession) handlePing() error {
 	err := sess.proto.WriteMessage(protocol.PingPongMsg{Type: "pong"})
 	if err == nil {
 		sess.Log.Debugf("ping.")
@@ -429,7 +474,7 @@
 	return err
 }
 
-func (sess *ClientSession) decodeBroadcast(bcast *serverMsg) *BroadcastNotification {
+func (sess *clientSession) decodeBroadcast(bcast *serverMsg) *BroadcastNotification {
 	decoded := make([]map[string]interface{}, 0)
 	for _, p := range bcast.Payloads {
 		var v map[string]interface{}
@@ -447,7 +492,7 @@
 }
 
 // handle "broadcast" messages
-func (sess *ClientSession) handleBroadcast(bcast *serverMsg) error {
+func (sess *clientSession) handleBroadcast(bcast *serverMsg) error {
 	err := sess.SeenState.SetLevel(bcast.ChanId, bcast.TopLevel)
 	if err != nil {
 		sess.setState(Error)
@@ -478,7 +523,7 @@
 }
 
 // handle "notifications" messages
-func (sess *ClientSession) handleNotifications(ucast *serverMsg) error {
+func (sess *clientSession) handleNotifications(ucast *serverMsg) error {
 	notifs, err := sess.SeenState.FilterBySeen(ucast.Notifications)
 	if err != nil {
 		sess.setState(Error)
@@ -512,7 +557,7 @@
 }
 
 // handle "connbroken" messages
-func (sess *ClientSession) handleConnBroken(connBroken *serverMsg) error {
+func (sess *clientSession) handleConnBroken(connBroken *serverMsg) error {
 	sess.setState(Error)
 	reason := connBroken.Reason
 	err := fmt.Errorf("server broke connection: %s", reason)
@@ -525,7 +570,7 @@
 }
 
 // handle "setparams" messages
-func (sess *ClientSession) handleSetParams(setParams *serverMsg) error {
+func (sess *clientSession) handleSetParams(setParams *serverMsg) error {
 	if setParams.SetCookie != "" {
 		sess.setCookie(setParams.SetCookie)
 	}
@@ -533,7 +578,7 @@
 }
 
 // loop runs the session with the server, emits a stream of events.
-func (sess *ClientSession) loop() error {
+func (sess *clientSession) loop() error {
 	var err error
 	var recv serverMsg
 	sess.setState(Running)
@@ -571,7 +616,7 @@
 }
 
 // Call this when you've connected and want to start looping.
-func (sess *ClientSession) start() error {
+func (sess *clientSession) start() error {
 	conn := sess.getConnection()
 	err := conn.SetDeadline(time.Now().Add(sess.ExchangeTimeout))
 	if err != nil {
@@ -634,8 +679,8 @@
 
 // run calls connect, and if it works it calls start, and if it works
 // it runs loop in a goroutine, and ships its return value over ErrCh.
-func (sess *ClientSession) run(closer func(), authChecker, hostGetter, connecter, starter, looper func() error) error {
-	closer()
+func (sess *clientSession) run(closer func(bool), authChecker, hostGetter, connecter, starter, looper func() error) error {
+	closer(false)
 	if err := authChecker(); err != nil {
 		return err
 	}
@@ -646,17 +691,14 @@
 	if err == nil {
 		err = starter()
 		if err == nil {
-			sess.ErrCh = make(chan error, 1)
-			sess.BroadcastCh = make(chan *BroadcastNotification)
-			sess.NotificationsCh = make(chan AddressedNotification)
-			go func() { sess.ErrCh <- looper() }()
+			go func() { sess.errCh <- looper() }()
 		}
 	}
 	return err
 }
 
 // This Jitter returns a random time.Duration somewhere in [-spread, spread].
-func (sess *ClientSession) Jitter(spread time.Duration) time.Duration {
+func (sess *clientSession) Jitter(spread time.Duration) time.Duration {
 	if spread < 0 {
 		panic("spread must be non-negative")
 	}
@@ -666,7 +708,7 @@
 
 // Dial takes the session from newly created (or newly disconnected)
 // to running the main loop.
-func (sess *ClientSession) Dial() error {
+func (sess *clientSession) Dial() error {
 	if sess.Protocolator == nil {
 		// a missing protocolator means you've willfully overridden
 		// it; returning an error here would prompt AutoRedial to just
@@ -676,6 +718,90 @@
 	return sess.run(sess.doClose, sess.addAuthorization, sess.getHosts, sess.connect, sess.start, sess.loop)
 }
 
+func (sess *clientSession) shutdown() {
+	sess.Log.Infof("session shutting down.")
+	sess.connLock.Lock()
+	defer sess.connLock.Unlock()
+	sess.stopRedial()
+	sess.closeConnection()
+}
+
+func (sess *clientSession) doKeepConnection() {
+	for {
+		select {
+		case cmd := <-sess.cmdCh:
+			switch cmd {
+			case cmdConnect:
+				sess.connHandler(true)
+			case cmdDisconnect:
+				sess.connHandler(false)
+			case cmdResetCookie:
+				sess.resetCookie()
+			}
+		case <-sess.stopCh:
+			sess.shutdown()
+			return
+		case n := <-sess.doneCh:
+			// if n == 0, the redialer aborted. If you do
+			// anything other than log it, keep that in mind.
+			sess.Log.Debugf("connected after %d attempts.", n)
+		case err := <-sess.errCh:
+			sess.errHandler(err)
+		}
+	}
+}
+
+func (sess *clientSession) handleConn(hasConn bool) {
+	sess.lastConn = hasConn
+
+	// Note this does not depend on the current state!  That's because Dial
+	// starts with doClose, which gets you to Disconnected even if you're
+	// connected, and you can call Close when Disconnected without it
+	// losing its stuff.
+	if hasConn {
+		sess.autoRedial()
+	} else {
+		sess.stopRedial()
+		sess.doClose(false)
+	}
+}
+
+func (sess *clientSession) handleErr(err error) {
+	sess.Log.Errorf("session error'ed out with %v", err)
+	// State() == Error mostly defends interrupting an ongoing
+	// autoRedial if we went quickly already through hasConn =
+	// false => hasConn = true
+	if sess.State() == Error && sess.lastConn {
+		sess.autoRedial()
+	}
+}
+
+func (sess *clientSession) KeepConnection() error {
+	sess.stateLock.Lock()
+	defer sess.stateLock.Unlock()
+	if sess.state != Pristine {
+		return errors.New("don't call KeepConnection() on a non-pristine session.")
+	}
+	sess.state = Disconnected
+
+	go sess.doKeepConnection()
+
+	return nil
+}
+
+func (sess *clientSession) StopKeepConnection() {
+	sess.setState(Shutdown)
+	close(sess.stopCh)
+}
+
+func (sess *clientSession) HasConnectivity(hasConn bool) {
+	if hasConn {
+		sess.cmdCh <- cmdConnect
+	} else {
+		sess.cmdCh <- cmdDisconnect
+	}
+}
+
 func init() {
 	rand.Seed(time.Now().Unix()) // good enough for us (we're not using it for crypto)
 }

=== modified file 'client/session/session_test.go'
--- client/session/session_test.go	2015-02-02 16:49:46 +0000
+++ client/session/session_test.go	2015-04-02 09:52:21 +0000
@@ -190,6 +190,24 @@
 	cs.lvls = func() (seenstate.SeenState, error) { return seenstate.NewSqliteSeenState(":memory:") }
 }
 
+func (cs *clientSessionSuite) TestStateString(c *C) {
+	for _, i := range []struct {
+		v ClientSessionState
+		s string
+	}{
+		{Error, "Error"},
+		{Pristine, "Pristine"},
+		{Disconnected, "Disconnected"},
+		{Connected, "Connected"},
+		{Started, "Started"},
+		{Running, "Running"},
+		{Shutdown, "Shutdown"},
+		{Unknown, fmt.Sprintf("??? (%d)", Unknown)},
+	} {
+		c.Check(i.v.String(), Equals, i.s)
+	}
+}
+
 /****************************************************************
   parseServerAddrSpec() tests
 ****************************************************************/
@@ -212,10 +230,15 @@
   NewSession() tests
 ****************************************************************/
 
-var dummyConf = ClientSessionConfig{}
+func dummyConf() ClientSessionConfig {
+	return ClientSessionConfig{
+		BroadcastCh:     make(chan *BroadcastNotification, 5),
+		NotificationsCh: make(chan AddressedNotification, 5),
+	}
+}
 
 func (cs *clientSessionSuite) TestNewSessionPlainWorks(c *C) {
-	sess, err := NewSession("foo:443", dummyConf, "", cs.lvls, cs.log)
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
 	c.Check(sess, NotNil)
 	c.Check(err, IsNil)
 	c.Check(sess.fallbackHosts, DeepEquals, []string{"foo:443"})
@@ -225,11 +248,13 @@
 	c.Check(sess.redialDelays, DeepEquals, util.Timeouts())
 	// but no root CAs set
 	c.Check(sess.TLS.RootCAs, IsNil)
-	c.Check(sess.State(), Equals, Disconnected)
+	c.Check(sess.State(), Equals, Pristine)
+	c.Check(sess.stopCh, NotNil)
+	c.Check(sess.cmdCh, NotNil)
 }
 
 func (cs *clientSessionSuite) TestNewSessionHostEndpointWorks(c *C) {
-	sess, err := NewSession("http://foo/hosts", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("http://foo/hosts", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	c.Check(sess.getHost, NotNil)
 }
@@ -255,7 +280,7 @@
 
 func (cs *clientSessionSuite) TestNewSessionBadSeenStateFails(c *C) {
 	ferr := func() (seenstate.SeenState, error) { return nil, errors.New("Busted.") }
-	sess, err := NewSession("", dummyConf, "wah", ferr, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", ferr, cs.log)
 	c.Check(sess, IsNil)
 	c.Assert(err, NotNil)
 }
@@ -266,7 +291,7 @@
 
 func (cs *clientSessionSuite) TestGetHostsFallback(c *C) {
 	fallback := []string{"foo:443", "bar:443"}
-	sess := &ClientSession{fallbackHosts: fallback}
+	sess := &clientSession{fallbackHosts: fallback}
 	err := sess.getHosts()
 	c.Assert(err, IsNil)
 	c.Check(sess.deliveryHosts, DeepEquals, fallback)
@@ -284,14 +309,14 @@
 
 func (cs *clientSessionSuite) TestGetHostsRemote(c *C) {
 	hostGetter := &testHostGetter{"example.com", []string{"foo:443", "bar:443"}, nil}
-	sess := &ClientSession{getHost: hostGetter, timeSince: time.Since}
+	sess := &clientSession{getHost: hostGetter, timeSince: time.Since}
 	err := sess.getHosts()
 	c.Assert(err, IsNil)
 	c.Check(sess.deliveryHosts, DeepEquals, []string{"foo:443", "bar:443"})
 }
 
 func (cs *clientSessionSuite) TestGetHostsRemoteError(c *C) {
-	sess, err := NewSession("", dummyConf, "", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	hostsErr := errors.New("failed")
 	hostGetter := &testHostGetter{"", nil, hostsErr}
@@ -304,7 +329,7 @@
 
 func (cs *clientSessionSuite) TestGetHostsRemoteCaching(c *C) {
 	hostGetter := &testHostGetter{"example.com", []string{"foo:443", "bar:443"}, nil}
-	sess := &ClientSession{
+	sess := &clientSession{
 		getHost: hostGetter,
 		ClientSessionConfig: ClientSessionConfig{
 			HostsCachingExpiryTime: 2 * time.Hour,
@@ -329,7 +354,7 @@
 
 func (cs *clientSessionSuite) TestGetHostsRemoteCachingReset(c *C) {
 	hostGetter := &testHostGetter{"example.com", []string{"foo:443", "bar:443"}, nil}
-	sess := &ClientSession{
+	sess := &clientSession{
 		getHost: hostGetter,
 		ClientSessionConfig: ClientSessionConfig{
 			HostsCachingExpiryTime: 2 * time.Hour,
@@ -356,7 +381,7 @@
 
 func (cs *clientSessionSuite) TestAddAuthorizationAddsAuthorization(c *C) {
 	url := "xyzzy://"
-	sess := &ClientSession{Log: cs.log}
+	sess := &clientSession{Log: cs.log}
 	sess.AuthGetter = func(url string) string { return url + " auth'ed" }
 	sess.AuthURL = url
 	c.Assert(sess.auth, Equals, "")
@@ -366,7 +391,7 @@
 }
 
 func (cs *clientSessionSuite) TestAddAuthorizationSkipsIfUnset(c *C) {
-	sess := &ClientSession{Log: cs.log}
+	sess := &clientSession{Log: cs.log}
 	sess.AuthGetter = nil
 	c.Assert(sess.auth, Equals, "")
 	err := sess.addAuthorization()
@@ -380,7 +405,7 @@
 
 func (cs *clientSessionSuite) TestStartConnectionAttempt(c *C) {
 	since := time.Since(time.Time{})
-	sess := &ClientSession{
+	sess := &clientSession{
 		ClientSessionConfig: ClientSessionConfig{
 			ExpectAllRepairedTime: 10 * time.Second,
 		},
@@ -404,7 +429,7 @@
 
 func (cs *clientSessionSuite) TestStartConnectionAttemptNoHostsPanic(c *C) {
 	since := time.Since(time.Time{})
-	sess := &ClientSession{
+	sess := &clientSession{
 		ClientSessionConfig: ClientSessionConfig{
 			ExpectAllRepairedTime: 10 * time.Second,
 		},
@@ -416,7 +441,7 @@
 }
 
 func (cs *clientSessionSuite) TestNextHostToTry(c *C) {
-	sess := &ClientSession{
+	sess := &clientSession{
 		deliveryHosts: []string{"foo:443", "bar:443", "baz:443"},
 		tryHost:       0,
 		leftToTry:     3,
@@ -439,7 +464,7 @@
 }
 
 func (cs *clientSessionSuite) TestStarted(c *C) {
-	sess, err := NewSession("", dummyConf, "", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 
 	sess.deliveryHosts = []string{"foo:443", "bar:443", "baz:443"}
@@ -458,7 +483,7 @@
 ****************************************************************/
 
 func (cs *clientSessionSuite) TestConnectFailsWithNoAddress(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.deliveryHosts = []string{"nowhere"}
 	sess.clearShouldDelay()
@@ -472,7 +497,7 @@
 	srv, err := net.Listen("tcp", "localhost:0")
 	c.Assert(err, IsNil)
 	defer srv.Close()
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.deliveryHosts = []string{srv.Addr().String()}
 	sess.clearShouldDelay()
@@ -487,7 +512,7 @@
 	srv, err := net.Listen("tcp", "localhost:0")
 	c.Assert(err, IsNil)
 	defer srv.Close()
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.deliveryHosts = []string{"nowhere", srv.Addr().String()}
 	sess.clearShouldDelay()
@@ -502,7 +527,7 @@
 func (cs *clientSessionSuite) TestConnectConnectFail(c *C) {
 	srv, err := net.Listen("tcp", "localhost:0")
 	c.Assert(err, IsNil)
-	sess, err := NewSession(srv.Addr().String(), dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession(srv.Addr().String(), dummyConf(), "wah", cs.lvls, cs.log)
 	srv.Close()
 	c.Assert(err, IsNil)
 	sess.deliveryHosts = []string{srv.Addr().String()}
@@ -513,101 +538,58 @@
 	c.Check(sess.State(), Equals, Error)
 }
 
-/****************************************************************
-  Close() tests
-****************************************************************/
-
-func (cs *clientSessionSuite) TestClose(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
-	c.Assert(err, IsNil)
-	sess.Connection = &testConn{Name: "TestClose"}
-	sess.Close()
-	c.Check(sess.Connection, IsNil)
-	c.Check(sess.State(), Equals, Disconnected)
-}
-
-func (cs *clientSessionSuite) TestCloseTwice(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
-	c.Assert(err, IsNil)
-	sess.Connection = &testConn{Name: "TestCloseTwice"}
-	sess.Close()
-	c.Check(sess.Connection, IsNil)
-	sess.Close()
-	c.Check(sess.Connection, IsNil)
-	c.Check(sess.State(), Equals, Disconnected)
-}
-
-func (cs *clientSessionSuite) TestCloseFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
-	c.Assert(err, IsNil)
-	sess.Connection = &testConn{Name: "TestCloseFails", CloseCondition: condition.Work(false)}
-	sess.Close()
-	c.Check(sess.Connection, IsNil) // nothing you can do to clean up anyway
-	c.Check(sess.State(), Equals, Disconnected)
-}
-
-type derp struct{ stopped bool }
-
-func (*derp) Redial() uint32 { return 0 }
-func (d *derp) Stop()        { d.stopped = true }
-
-func (cs *clientSessionSuite) TestCloseStopsRetrier(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
-	c.Assert(err, IsNil)
-	ar := new(derp)
-	sess.retrier = ar
-	c.Check(ar.stopped, Equals, false)
-	sess.Close()
-	c.Check(ar.stopped, Equals, true)
-	sess.Close() // double close check
-	c.Check(ar.stopped, Equals, true)
-}
-
-/****************************************************************
-  AutoRedial() tests
-****************************************************************/
+type dumbRetrier struct{ stopped bool }
+
+func (*dumbRetrier) Redial() uint32 { return 0 }
+func (d *dumbRetrier) Stop()        { d.stopped = true }
+
+// /****************************************************************
+//   AutoRedial() tests
+// ****************************************************************/
 
 func (cs *clientSessionSuite) TestAutoRedialWorks(c *C) {
 	// checks that AutoRedial sets up a retrier and tries redialing it
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
-	ar := new(derp)
+	ar := new(dumbRetrier)
 	sess.retrier = ar
 	c.Check(ar.stopped, Equals, false)
-	sess.AutoRedial(nil)
+	sess.autoRedial()
+	defer sess.stopRedial()
 	c.Check(ar.stopped, Equals, true)
 }
 
 func (cs *clientSessionSuite) TestAutoRedialStopsRetrier(c *C) {
 	// checks that AutoRedial stops the previous retrier
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
-	ch := make(chan uint32)
+	sess.doneCh = make(chan uint32)
 	c.Check(sess.retrier, IsNil)
-	sess.AutoRedial(ch)
+	sess.autoRedial()
 	c.Assert(sess.retrier, NotNil)
 	sess.retrier.Stop()
-	c.Check(<-ch, Not(Equals), 0)
+	c.Check(<-sess.doneCh, Not(Equals), 0)
 }
 
 func (cs *clientSessionSuite) TestAutoRedialCallsRedialDelay(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	// NOTE there are tests that use calling redialDelay as an indication of calling autoRedial!
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	flag := false
-	sess.redialDelay = func(sess *ClientSession) time.Duration { flag = true; return 0 }
-	sess.AutoRedial(nil)
+	sess.redialDelay = func(sess *clientSession) time.Duration { flag = true; return 0 }
+	sess.autoRedial()
 	c.Check(flag, Equals, true)
 }
 
 func (cs *clientSessionSuite) TestAutoRedialSetsRedialDelayIfTooQuick(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
-	sess.redialDelay = func(sess *ClientSession) time.Duration { return 0 }
-	sess.AutoRedial(nil)
+	sess.redialDelay = func(sess *clientSession) time.Duration { return 0 }
+	sess.autoRedial()
 	c.Check(sess.ShouldDelay(), Equals, false)
 	sess.stopRedial()
 	sess.clearShouldDelay()
-	sess.AutoRedial(nil)
+	sess.autoRedial()
 	c.Check(sess.ShouldDelay(), Equals, true)
 }
 
@@ -616,29 +598,23 @@
 ****************************************************************/
 
 type msgSuite struct {
-	sess   *ClientSession
+	sess   *clientSession
 	upCh   chan interface{}
 	downCh chan interface{}
-	errCh  chan error
 }
 
 var _ = Suite(&msgSuite{})
 
 func (s *msgSuite) SetUpTest(c *C) {
 	var err error
-	conf := ClientSessionConfig{
-		ExchangeTimeout: time.Millisecond,
-	}
+	conf := dummyConf()
+	conf.ExchangeTimeout = time.Millisecond
 	s.sess, err = NewSession("", conf, "wah", seenstate.NewSeenState, helpers.NewTestLogger(c, "debug"))
 	c.Assert(err, IsNil)
 	s.sess.Connection = &testConn{Name: "TestHandle*"}
-	s.errCh = make(chan error, 1)
 	s.upCh = make(chan interface{}, 5)
 	s.downCh = make(chan interface{}, 5)
 	s.sess.proto = &testProtocol{up: s.upCh, down: s.downCh}
-	// make the message channel buffered
-	s.sess.BroadcastCh = make(chan *BroadcastNotification, 5)
-	s.sess.NotificationsCh = make(chan AddressedNotification, 5)
 }
 
 func (s *msgSuite) TestHandlePingWorks(c *C) {
@@ -694,10 +670,10 @@
 			json.RawMessage(`{"img1/m1":[102,"tubular"]}`),
 		},
 	}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, Equals, nil)
+	c.Check(<-s.sess.errCh, Equals, nil)
 	c.Assert(len(s.sess.BroadcastCh), Equals, 1)
 	c.Check(<-s.sess.BroadcastCh, DeepEquals, &BroadcastNotification{
 		TopLevel: 2,
@@ -726,11 +702,11 @@
 		TopLevel: 2,
 		Payloads: []json.RawMessage{json.RawMessage(`{"b":1}`)},
 	}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	failure := errors.New("ACK ACK ACK")
 	s.upCh <- failure
-	c.Assert(<-s.errCh, Equals, failure)
+	c.Assert(<-s.sess.errCh, Equals, failure)
 	c.Check(s.sess.State(), Equals, Error)
 }
 
@@ -744,10 +720,10 @@
 		TopLevel: 2,
 		Payloads: []json.RawMessage{json.RawMessage(`{"b":1}`)},
 	}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, IsNil)
+	c.Check(<-s.sess.errCh, IsNil)
 	c.Check(len(s.sess.BroadcastCh), Equals, 0)
 }
 
@@ -762,10 +738,10 @@
 		TopLevel: 2,
 		Payloads: []json.RawMessage{json.RawMessage(`{"b":1}`)},
 	}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	s.upCh <- nil // ack ok
 	// start returns with error
-	c.Check(<-s.errCh, Not(Equals), nil)
+	c.Check(<-s.sess.errCh, Not(Equals), nil)
 	c.Check(s.sess.State(), Equals, Error)
 	// no message sent out
 	c.Check(len(s.sess.BroadcastCh), Equals, 0)
@@ -778,10 +754,10 @@
 	s.sess.setShouldDelay()
 
 	msg := &serverMsg{Type: "broadcast"}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, IsNil)
+	c.Check(<-s.sess.errCh, IsNil)
 
 	c.Check(s.sess.ShouldDelay(), Equals, false)
 }
@@ -790,10 +766,10 @@
 	s.sess.setShouldDelay()
 
 	msg := &serverMsg{Type: "broadcast"}
-	go func() { s.errCh <- s.sess.handleBroadcast(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleBroadcast(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- errors.New("bcast")
-	c.Check(<-s.errCh, NotNil)
+	c.Check(<-s.sess.errCh, NotNil)
 
 	c.Check(s.sess.ShouldDelay(), Equals, true)
 }
@@ -843,10 +819,10 @@
 	msg.NotificationsMsg = protocol.NotificationsMsg{
 		Notifications: []protocol.Notification{n1, n2},
 	}
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, Equals, nil)
+	c.Check(<-s.sess.errCh, Equals, nil)
 	c.Check(s.sess.ShouldDelay(), Equals, false)
 	c.Assert(s.sess.NotificationsCh, HasLen, 2)
 	app1, err := click.ParseAppId("com.example.app1_app1")
@@ -889,10 +865,10 @@
 	msg.NotificationsMsg = protocol.NotificationsMsg{
 		Notifications: []protocol.Notification{n1, n2},
 	}
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, Equals, nil)
+	c.Check(<-s.sess.errCh, Equals, nil)
 	c.Check(s.sess.ShouldDelay(), Equals, false)
 	c.Assert(s.sess.NotificationsCh, HasLen, 1)
 	app2, err := click.ParseAppId("com.example.app2_app2")
@@ -924,10 +900,10 @@
 	msg.NotificationsMsg = protocol.NotificationsMsg{
 		Notifications: []protocol.Notification{n1, n2},
 	}
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, Equals, nil)
+	c.Check(<-s.sess.errCh, Equals, nil)
 	c.Assert(s.sess.NotificationsCh, HasLen, 2)
 	app1, err := click.ParseAppId("com.example.app1_app1")
 	c.Assert(err, IsNil)
@@ -944,10 +920,10 @@
 	c.Check(ac.ops, HasLen, 3)
 
 	// second time they get ignored
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	s.upCh <- nil // ack ok
-	c.Check(<-s.errCh, Equals, nil)
+	c.Check(<-s.sess.errCh, Equals, nil)
 	c.Assert(s.sess.NotificationsCh, HasLen, 0)
 	c.Check(ac.ops, HasLen, 4)
 }
@@ -964,11 +940,11 @@
 	msg.NotificationsMsg = protocol.NotificationsMsg{
 		Notifications: []protocol.Notification{n1},
 	}
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	failure := errors.New("ACK ACK ACK")
 	s.upCh <- failure
-	c.Assert(<-s.errCh, Equals, failure)
+	c.Assert(<-s.sess.errCh, Equals, failure)
 	c.Check(s.sess.State(), Equals, Error)
 	// didn't get to clear
 	c.Check(s.sess.ShouldDelay(), Equals, true)
@@ -987,10 +963,10 @@
 	msg.NotificationsMsg = protocol.NotificationsMsg{
 		Notifications: []protocol.Notification{n1},
 	}
-	go func() { s.errCh <- s.sess.handleNotifications(msg) }()
+	go func() { s.sess.errCh <- s.sess.handleNotifications(msg) }()
 	s.upCh <- nil // ack ok
 	// start returns with error
-	c.Check(<-s.errCh, Not(Equals), nil)
+	c.Check(<-s.sess.errCh, Not(Equals), nil)
 	c.Check(s.sess.State(), Equals, Error)
 	// no message sent out
 	c.Check(len(s.sess.NotificationsCh), Equals, 0)
@@ -1011,8 +987,8 @@
 	msg.ConnBrokenMsg = protocol.ConnBrokenMsg{
 		Reason: "REASON",
 	}
-	go func() { s.errCh <- s.sess.handleConnBroken(msg) }()
-	c.Check(<-s.errCh, ErrorMatches, "server broke connection: REASON")
+	go func() { s.sess.errCh <- s.sess.handleConnBroken(msg) }()
+	c.Check(<-s.sess.errCh, ErrorMatches, "server broke connection: REASON")
 	c.Check(s.sess.State(), Equals, Error)
 }
 
@@ -1023,8 +999,8 @@
 		Reason: protocol.BrokenHostMismatch,
 	}
 	s.sess.deliveryHosts = []string{"foo:443", "bar:443"}
-	go func() { s.errCh <- s.sess.handleConnBroken(msg) }()
-	c.Check(<-s.errCh, ErrorMatches, "server broke connection: host-mismatch")
+	go func() { s.sess.errCh <- s.sess.handleConnBroken(msg) }()
+	c.Check(<-s.sess.errCh, ErrorMatches, "server broke connection: host-mismatch")
 	c.Check(s.sess.State(), Equals, Error)
 	// hosts were reset
 	c.Check(s.sess.deliveryHosts, IsNil)
@@ -1042,14 +1018,14 @@
 	(*msgSuite)(s).SetUpTest(c)
 	s.sess.Connection.(*testConn).Name = "TestLoop*"
 	go func() {
-		s.errCh <- s.sess.loop()
+		s.sess.errCh <- s.sess.loop()
 	}()
 }
 
 func (s *loopSuite) TestLoopReadError(c *C) {
 	c.Check(s.sess.State(), Equals, Running)
 	s.upCh <- errors.New("Read")
-	err := <-s.errCh
+	err := <-s.sess.errCh
 	c.Check(err, ErrorMatches, "Read")
 	c.Check(s.sess.State(), Equals, Error)
 }
@@ -1061,7 +1037,7 @@
 	c.Check(takeNext(s.downCh), Equals, protocol.PingPongMsg{Type: "pong"})
 	failure := errors.New("pong")
 	s.upCh <- failure
-	c.Check(<-s.errCh, Equals, failure)
+	c.Check(<-s.sess.errCh, Equals, failure)
 }
 
 func (s *loopSuite) TestLoopLoopsDaLoop(c *C) {
@@ -1074,7 +1050,7 @@
 	}
 	failure := errors.New("pong")
 	s.upCh <- failure
-	c.Check(<-s.errCh, Equals, failure)
+	c.Check(<-s.sess.errCh, Equals, failure)
 }
 
 func (s *loopSuite) TestLoopBroadcast(c *C) {
@@ -1091,7 +1067,7 @@
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	failure := errors.New("ack")
 	s.upCh <- failure
-	c.Check(<-s.errCh, Equals, failure)
+	c.Check(<-s.sess.errCh, Equals, failure)
 }
 
 func (s *loopSuite) TestLoopNotifications(c *C) {
@@ -1111,7 +1087,7 @@
 	c.Check(takeNext(s.downCh), Equals, protocol.AckMsg{"ack"})
 	failure := errors.New("ack")
 	s.upCh <- failure
-	c.Check(<-s.errCh, Equals, failure)
+	c.Check(<-s.sess.errCh, Equals, failure)
 }
 
 func (s *loopSuite) TestLoopSetParams(c *C) {
@@ -1124,7 +1100,7 @@
 	s.upCh <- setParams
 	failure := errors.New("fail")
 	s.upCh <- failure
-	c.Assert(<-s.errCh, Equals, failure)
+	c.Assert(<-s.sess.errCh, Equals, failure)
 	c.Check(s.sess.getCookie(), Equals, "COOKIE")
 }
 
@@ -1136,7 +1112,7 @@
 	}
 	c.Check(takeNext(s.downCh), Equals, "deadline 1ms")
 	s.upCh <- broken
-	c.Check(<-s.errCh, NotNil)
+	c.Check(<-s.sess.errCh, NotNil)
 }
 
 func (s *loopSuite) TestLoopConnWarn(c *C) {
@@ -1157,7 +1133,7 @@
 	s.upCh <- warn
 	s.upCh <- connwarn
 	s.upCh <- failure
-	c.Check(<-s.errCh, Equals, failure)
+	c.Check(<-s.sess.errCh, Equals, failure)
 	c.Check(log.Captured(),
 		Matches, `(?ms).* warning: XXX$.*`)
 	c.Check(log.Captured(),
@@ -1168,7 +1144,7 @@
   start() tests
 ****************************************************************/
 func (cs *clientSessionSuite) TestStartFailsIfSetDeadlineFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartFailsIfSetDeadlineFails",
 		DeadlineCondition: condition.Work(false)} // setdeadline will fail
@@ -1178,7 +1154,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartFailsIfWriteFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartFailsIfWriteFails",
 		WriteCondition: condition.Work(false)} // write will fail
@@ -1188,7 +1164,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartFailsIfGetLevelsFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.SeenState = &brokenSeenState{}
 	sess.Connection = &testConn{Name: "TestStartConnectMessageFails"}
@@ -1208,7 +1184,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartConnectMessageFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartConnectMessageFails"}
 	errCh := make(chan error, 1)
@@ -1235,7 +1211,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartConnackReadError(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartConnackReadError"}
 	errCh := make(chan error, 1)
@@ -1259,7 +1235,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartBadConnack(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartBadConnack"}
 	errCh := make(chan error, 1)
@@ -1283,7 +1259,7 @@
 }
 
 func (cs *clientSessionSuite) TestStartNotConnack(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Connection = &testConn{Name: "TestStartBadConnack"}
 	errCh := make(chan error, 1)
@@ -1350,13 +1326,31 @@
   run() tests
 ****************************************************************/
 
+func (cs *clientSessionSuite) TestRunCallsCloserWithFalse(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	failure := errors.New("bail")
+	has_closed := false
+	with_false := false
+	err = sess.run(
+		func(b bool) { has_closed = true; with_false = !b },
+		func() error { return failure },
+		nil,
+		nil,
+		nil,
+		nil)
+	c.Check(err, Equals, failure)
+	c.Check(has_closed, Equals, true)
+	c.Check(with_false, Equals, true)
+}
+
 func (cs *clientSessionSuite) TestRunBailsIfAuthCheckFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	failure := errors.New("TestRunBailsIfAuthCheckFails")
 	has_closed := false
 	err = sess.run(
-		func() { has_closed = true },
+		func(bool) { has_closed = true },
 		func() error { return failure },
 		nil,
 		nil,
@@ -1367,12 +1361,12 @@
 }
 
 func (cs *clientSessionSuite) TestRunBailsIfHostGetterFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	failure := errors.New("TestRunBailsIfHostGetterFails")
 	has_closed := false
 	err = sess.run(
-		func() { has_closed = true },
+		func(bool) { has_closed = true },
 		func() error { return nil },
 		func() error { return failure },
 		nil,
@@ -1383,11 +1377,11 @@
 }
 
 func (cs *clientSessionSuite) TestRunBailsIfConnectFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	failure := errors.New("TestRunBailsIfConnectFails")
 	err = sess.run(
-		func() {},
+		func(bool) {},
 		func() error { return nil },
 		func() error { return nil },
 		func() error { return failure },
@@ -1397,11 +1391,11 @@
 }
 
 func (cs *clientSessionSuite) TestRunBailsIfStartFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	failure := errors.New("TestRunBailsIfStartFails")
 	err = sess.run(
-		func() {},
+		func(bool) {},
 		func() error { return nil },
 		func() error { return nil },
 		func() error { return nil },
@@ -1411,16 +1405,12 @@
 }
 
 func (cs *clientSessionSuite) TestRunRunsEvenIfLoopFails(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
-	// just to make a point: until here we haven't set ErrCh & BroadcastCh (no
-	// biggie if this stops being true)
-	c.Check(sess.ErrCh, IsNil)
-	c.Check(sess.BroadcastCh, IsNil)
 	failureCh := make(chan error) // must be unbuffered
 	notf := &BroadcastNotification{}
 	err = sess.run(
-		func() {},
+		func(bool) {},
 		func() error { return nil },
 		func() error { return nil },
 		func() error { return nil },
@@ -1428,12 +1418,12 @@
 		func() error { sess.BroadcastCh <- notf; return <-failureCh })
 	c.Check(err, Equals, nil)
 	// if run doesn't error it sets up the channels
-	c.Assert(sess.ErrCh, NotNil)
+	c.Assert(sess.errCh, NotNil)
 	c.Assert(sess.BroadcastCh, NotNil)
 	c.Check(<-sess.BroadcastCh, Equals, notf)
 	failure := errors.New("TestRunRunsEvenIfLoopFails")
 	failureCh <- failure
-	c.Check(<-sess.ErrCh, Equals, failure)
+	c.Check(<-sess.errCh, Equals, failure)
 	// so now you know it was running in a goroutine :)
 }
 
@@ -1442,7 +1432,7 @@
 ****************************************************************/
 
 func (cs *clientSessionSuite) TestJitter(c *C) {
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	num_tries := 20       // should do the math
 	spread := time.Second //
@@ -1474,7 +1464,7 @@
 
 func (cs *clientSessionSuite) TestDialPanics(c *C) {
 	// one last unhappy test
-	sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.Protocolator = nil
 	c.Check(sess.Dial, PanicMatches, ".*protocol constructor.")
@@ -1482,12 +1472,15 @@
 
 var (
 	dialTestTimeout = 300 * time.Millisecond
-	dialTestConf    = ClientSessionConfig{
-		ExchangeTimeout: dialTestTimeout,
-		PEM:             helpers.TestCertPEMBlock,
-	}
 )
 
+func dialTestConf() ClientSessionConfig {
+	conf := dummyConf()
+	conf.ExchangeTimeout = dialTestTimeout
+	conf.PEM = helpers.TestCertPEMBlock
+	return conf
+}
+
 func (cs *clientSessionSuite) TestDialBadServerName(c *C) {
 	// a borked server name
 	lst, err := tls.Listen("tcp", "localhost:0", helpers.TestTLSServerConfig)
@@ -1506,7 +1499,7 @@
 	}))
 	defer ts.Close()
 
-	sess, err := NewSession(ts.URL, dialTestConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession(ts.URL, dialTestConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	tconn := &testConn{}
 	sess.Connection = tconn
@@ -1551,7 +1544,7 @@
 	}))
 	defer ts.Close()
 
-	sess, err := NewSession(ts.URL, dialTestConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession(ts.URL, dialTestConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	tconn := &testConn{CloseCondition: condition.Fail2Work(10)}
 	sess.Connection = tconn
@@ -1631,16 +1624,16 @@
 	c.Check(takeNext(downCh), Equals, protocol.PingPongMsg{Type: "pong"})
 	failure := errors.New("pongs")
 	upCh <- failure
-	c.Check(<-sess.ErrCh, Equals, failure)
+	c.Check(<-sess.errCh, Equals, failure)
 }
 
 func (cs *clientSessionSuite) TestDialWorksDirect(c *C) {
 	// happy path thoughts
 	lst, err := tls.Listen("tcp", "localhost:0", helpers.TestTLSServerConfig)
 	c.Assert(err, IsNil)
-	sess, err := NewSession(lst.Addr().String(), dialTestConf, "wah", cs.lvls, cs.log)
+	sess, err := NewSession(lst.Addr().String(), dialTestConf(), "wah", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
-	defer sess.Close()
+	defer sess.StopKeepConnection()
 
 	upCh := make(chan interface{}, 5)
 	downCh := make(chan interface{}, 5)
@@ -1659,7 +1652,7 @@
 ****************************************************************/
 
 func (cs *clientSessionSuite) TestShouldDelay(c *C) {
-	sess, err := NewSession("foo:443", dummyConf, "", cs.lvls, cs.log)
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	c.Check(sess.ShouldDelay(), Equals, false)
 	sess.setShouldDelay()
@@ -1669,7 +1662,7 @@
 }
 
 func (cs *clientSessionSuite) TestRedialDelay(c *C) {
-	sess, err := NewSession("foo:443", dummyConf, "", cs.lvls, cs.log)
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
 	sess.redialDelays = []time.Duration{17, 42}
 	n := 0
@@ -1690,15 +1683,207 @@
 }
 
 /****************************************************************
-  ClearCookie() tests
+  ResetCookie() tests
 ****************************************************************/
 
-func (cs *clientSessionSuite) TestClearCookie(c *C) {
-	sess, err := NewSession("foo:443", dummyConf, "", cs.lvls, cs.log)
+func (cs *clientSessionSuite) TestResetCookie(c *C) {
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
 	c.Assert(err, IsNil)
+	c.Assert(sess.KeepConnection(), IsNil)
+	defer sess.StopKeepConnection()
 	c.Check(sess.getCookie(), Equals, "")
 	sess.setCookie("COOKIE")
 	c.Check(sess.getCookie(), Equals, "COOKIE")
-	sess.ClearCookie()
+	sess.ResetCookie()
 	c.Check(sess.getCookie(), Equals, "")
 }
+
+/****************************************************************
+  KeepConnection() (and related) tests
+****************************************************************/
+
+func (cs *clientSessionSuite) TestKeepConnectionDoesNothingIfNotConnected(c *C) {
+	// how do you test "does nothing?"
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	c.Assert(sess, NotNil)
+	c.Assert(sess.State(), Equals, Pristine)
+	c.Assert(sess.KeepConnection(), IsNil)
+	defer sess.StopKeepConnection()
+	// stopCh is meant to be used just for closing it, but abusing
+	// it for testing seems the right thing to do: this ensures
+	// the thing is ticking along before we check the state of
+	// stuff.
+	sess.stopCh <- struct{}{}
+	c.Check(sess.State(), Equals, Disconnected)
+}
+
+func (cs *clientSessionSuite) TestYouCantCallKeepConnectionTwice(c *C) {
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	c.Assert(sess, NotNil)
+	c.Assert(sess.State(), Equals, Pristine)
+	c.Assert(sess.KeepConnection(), IsNil)
+	defer sess.StopKeepConnection()
+	c.Check(sess.KeepConnection(), NotNil)
+}
+
+func (cs *clientSessionSuite) TestStopKeepConnectionShutsdown(c *C) {
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	c.Assert(sess, NotNil)
+	sess.StopKeepConnection()
+	c.Check(sess.State(), Equals, Shutdown)
+}
+
+func (cs *clientSessionSuite) TestHasConnectivityTriggersConnectivityHandler(c *C) {
+	sess, err := NewSession("foo:443", dummyConf(), "", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	c.Assert(sess, NotNil)
+	testCh := make(chan bool)
+	sess.connHandler = func(p bool) { testCh <- p }
+	go sess.doKeepConnection()
+	defer sess.StopKeepConnection()
+	sess.HasConnectivity(true)
+	c.Check(<-testCh, Equals, true)
+	sess.HasConnectivity(false)
+	c.Check(<-testCh, Equals, false)
+}
+
+func (cs *clientSessionSuite) TestDoneChIsEmptiedAndLogged(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	sess.doneCh = make(chan uint32) // unbuffered
+
+	sess.KeepConnection()
+	defer sess.StopKeepConnection()
+
+	sess.doneCh <- 23
+	sess.doneCh <- 24 // makes sure the first one has been processed before checking
+
+	c.Check(cs.log.Captured(),
+		Matches, `(?ms).* connected after 23 attempts\.`)
+}
+
+func (cs *clientSessionSuite) TestErrChIsEmptiedAndLoggedAndAutoRedial(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.errCh = make(chan error) // unbuffered
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.lastConn = true // -> autoRedial, if the session is in Disconnected
+
+	sess.KeepConnection()
+	defer sess.StopKeepConnection()
+
+	sess.setState(Error)
+	sess.errCh <- errors.New("potato")
+	select {
+	case <-ch:
+		// all ok
+	case <-time.After(100 * time.Millisecond):
+		c.Fatalf("redialDelay not called (-> autoRedial not called)?")
+	}
+
+	c.Check(cs.log.Captured(),
+		Matches, `(?ms).* session error.*potato`)
+}
+
+func (cs *clientSessionSuite) TestErrChIsEmptiedAndLoggedNoAutoRedial(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.errCh = make(chan error) // unbuffered
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.connHandler = func(bool) {}
+	sess.lastConn = false // so, no autoredial
+
+	sess.KeepConnection()
+	defer sess.StopKeepConnection()
+
+	sess.errCh <- errors.New("potato")
+	c.Assert(sess.State(), Equals, Disconnected)
+	select {
+	case <-ch:
+		c.Fatalf("redialDelay called (-> autoRedial called) when disconnected?")
+	case <-time.After(100 * time.Millisecond):
+		// all ok
+	}
+
+	c.Check(cs.log.Captured(),
+		Matches, `(?ms).* session error.*potato`)
+}
+
+func (cs *clientSessionSuite) TestHandleConnConnFromConnected(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.state = Connected
+	sess.lastConn = true
+	sess.handleConn(true)
+	c.Check(sess.lastConn, Equals, true)
+
+	select {
+	case <-ch:
+		// all ok
+	case <-time.After(100 * time.Millisecond):
+		c.Fatalf("redialDelay not called (-> autoRedial not called)?")
+	}
+}
+
+func (cs *clientSessionSuite) TestHandleConnConnFromDisconnected(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.state = Disconnected
+	sess.lastConn = false
+	sess.handleConn(true)
+	c.Check(sess.lastConn, Equals, true)
+
+	select {
+	case <-ch:
+		// all ok
+	case <-time.After(100 * time.Millisecond):
+		c.Fatalf("redialDelay not called (-> autoRedial not called)?")
+	}
+}
+
+func (cs *clientSessionSuite) TestHandleConnNotConnFromDisconnected(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.state = Disconnected
+	sess.lastConn = false
+	sess.handleConn(false)
+	c.Check(sess.lastConn, Equals, false)
+
+	select {
+	case <-ch:
+		c.Fatalf("redialDelay called (-> autoRedial called)?")
+	case <-time.After(100 * time.Millisecond):
+		// all ok
+	}
+	c.Check(cs.log.Captured(), Matches, `(?ms).*-> Disconnected`)
+}
+
+func (cs *clientSessionSuite) TestHandleConnNotConnFromConnected(c *C) {
+	sess, err := NewSession("", dummyConf(), "wah", cs.lvls, cs.log)
+	c.Assert(err, IsNil)
+	ch := make(chan struct{}, 1)
+	sess.redialDelay = func(sess *clientSession) time.Duration { ch <- struct{}{}; return 0 }
+	sess.state = Connected
+	sess.lastConn = true
+	sess.handleConn(false)
+	c.Check(sess.lastConn, Equals, false)
+
+	select {
+	case <-ch:
+		c.Fatalf("redialDelay called (-> autoRedial called)?")
+	case <-time.After(100 * time.Millisecond):
+		// all ok
+	}
+	c.Check(cs.log.Captured(), Matches, `(?ms).*-> Disconnected`)
+}

=== modified file 'debian/changelog'
--- debian/changelog	2015-03-06 13:27:06 +0000
+++ debian/changelog	2015-04-02 09:52:21 +0000
@@ -1,3 +1,26 @@
+ubuntu-push (0.68+15.04.20150306.2-0ubuntu2) UNRELEASED; urgency=medium
+
+  [Roberto Alsina]
+  * click-hook: report failure if hooks_path doesn't exist. [client]
+
+  [Bret Barker]
+  * add a hacky busy sleep loop to workaround go's sleep not
+    accounting for suspended time, more logging (lp:1435109). [client]
+
+  [John R. Lenton]
+  * Refactor code maintaining session (better fix for lp:1390663) [client]
+
+  [Samuele Pedroni]
+  * just delegate whether there's a update-worthy image to the
+    system-settings helper and system-image. [client]
+  * stop waking up for polling if in flight-mode and wireless not
+    enabled (lp:1437135). [client]
+  * don't hold a lock for a long time on handleErrConn, trigger
+    autoRedial on Error more actively (lp:1435109). [client]
+  * disallow RC4 and SSLv3. [server]
+
+ -- Samuele Pedroni <pedronis@ubuntu-64>  Thu, 02 Apr 2015 11:38:19 +0200
+
 ubuntu-push (0.68+15.04.20150306.2-0ubuntu1) vivid; urgency=medium
 
   [ John R. Lenton ]

=== modified file 'debian/config.json'
--- debian/config.json	2015-01-26 21:00:27 +0000
+++ debian/config.json	2015-04-02 09:52:21 +0000
@@ -19,5 +19,6 @@
     "poll_settle":      "20ms",
     "poll_net_wait":    "1m",
     "poll_polld_wait":  "3m",
-    "poll_done_wait":   "5s"
+    "poll_done_wait":   "5s",
+    "poll_busy_wait":   "1s"
 }

=== modified file 'launch_helper/kindpool_test.go'
--- launch_helper/kindpool_test.go	2014-08-22 14:54:41 +0000
+++ launch_helper/kindpool_test.go	2015-04-02 09:52:21 +0000
@@ -45,7 +45,7 @@
 	select {
 	case res := <-ch:
 		return res
-	case <-time.After(100 * time.Millisecond):
+	case <-time.After(time.Second):
 		c.Fatal("timeout waiting for result")
 	}
 	return nil

=== modified file 'poller/poller.go'
--- poller/poller.go	2015-01-21 17:21:42 +0000
+++ poller/poller.go	2015-04-02 09:52:21 +0000
@@ -1,5 +1,5 @@
 /*
- Copyright 2014 Canonical Ltd.
+ Copyright 2014-2015 Canonical Ltd.
 
  This program is free software: you can redistribute it and/or modify it
  under the terms of the GNU General Public License version 3, as published
@@ -25,8 +25,10 @@
 	"time"
 
 	"launchpad.net/ubuntu-push/bus"
+	"launchpad.net/ubuntu-push/bus/networkmanager"
 	"launchpad.net/ubuntu-push/bus/polld"
 	"launchpad.net/ubuntu-push/bus/powerd"
+	"launchpad.net/ubuntu-push/bus/urfkill"
 	"launchpad.net/ubuntu-push/client/session"
 	"launchpad.net/ubuntu-push/logger"
 	"launchpad.net/ubuntu-push/util"
@@ -48,6 +50,7 @@
 	NetworkWait        time.Duration
 	PolldWait          time.Duration
 	DoneWait           time.Duration
+	BusyWait           time.Duration
 }
 
 type Poller interface {
@@ -63,21 +66,29 @@
 }
 
 type poller struct {
-	times        Times
-	log          logger.Logger
-	powerd       powerd.Powerd
-	polld        polld.Polld
-	cookie       string
-	sessionState stater
+	times                Times
+	log                  logger.Logger
+	nm                   networkmanager.NetworkManager
+	powerd               powerd.Powerd
+	polld                polld.Polld
+	urfkill              urfkill.URfkill
+	cookie               string
+	sessionState         stater
+	requestWakeupCh      chan struct{}
+	requestedWakeupErrCh chan error
+	holdsWakeLockCh      chan bool
 }
 
 func New(setup *PollerSetup) Poller {
 	return &poller{
-		times:        setup.Times,
-		log:          setup.Log,
-		powerd:       nil,
-		polld:        nil,
-		sessionState: setup.SessionStateGetter,
+		times:                setup.Times,
+		log:                  setup.Log,
+		powerd:               nil,
+		polld:                nil,
+		sessionState:         setup.SessionStateGetter,
+		requestWakeupCh:      make(chan struct{}),
+		requestedWakeupErrCh: make(chan error),
+		holdsWakeLockCh:      make(chan bool),
 	}
 }
 
@@ -92,10 +103,17 @@
 	if p.powerd != nil || p.polld != nil {
 		return ErrAlreadyStarted
 	}
+	nmEndp := bus.SystemBus.Endpoint(networkmanager.BusAddress, p.log)
 	powerdEndp := bus.SystemBus.Endpoint(powerd.BusAddress, p.log)
 	polldEndp := bus.SessionBus.Endpoint(polld.BusAddress, p.log)
+	urEndp := bus.SystemBus.Endpoint(urfkill.BusAddress, p.log)
 	var wg sync.WaitGroup
-	wg.Add(2)
+	wg.Add(4)
+	go func() {
+		n := util.NewAutoRedialer(nmEndp).Redial()
+		p.log.Debugf("NetworkManager dialed on try %d", n)
+		wg.Done()
+	}()
 	go func() {
 		n := util.NewAutoRedialer(powerdEndp).Redial()
 		p.log.Debugf("powerd dialed on try %d", n)
@@ -106,10 +124,31 @@
 		p.log.Debugf("polld dialed in on try %d", n)
 		wg.Done()
 	}()
+	go func() {
+		n := util.NewAutoRedialer(urEndp).Redial()
+		p.log.Debugf("URfkill dialed on try %d", n)
+		wg.Done()
+	}()
 	wg.Wait()
 
+	p.nm = networkmanager.New(nmEndp, p.log)
 	p.powerd = powerd.New(powerdEndp, p.log)
 	p.polld = polld.New(polldEndp, p.log)
+	p.urfkill = urfkill.New(urEndp, p.log)
+
+	// busy sleep loop to workaround go's timer/sleep
+	// not accounting for time when the system is suspended
+	// see https://bugs.launchpad.net/ubuntu/+source/ubuntu-push/+bug/1435109
+	if p.times.BusyWait > 0 {
+		p.log.Debugf("starting busy loop with %s interval", p.times.BusyWait)
+		go func() {
+			for {
+				time.Sleep(p.times.BusyWait)
+			}
+		}()
+	} else {
+		p.log.Debugf("skipping busy loop")
+	}
 
 	return nil
 }
@@ -118,7 +157,7 @@
 	if p.log == nil {
 		return ErrUnconfigured
 	}
-	if p.powerd == nil || p.polld == nil {
+	if p.nm == nil || p.powerd == nil || p.polld == nil || p.urfkill == nil {
 		return ErrNotStarted
 	}
 	wakeupCh, err := p.powerd.WatchWakeups()
@@ -129,10 +168,110 @@
 	if err != nil {
 		return err
 	}
-	go p.run(wakeupCh, doneCh)
+	flightMode := p.urfkill.IsFlightMode()
+	wirelessEnabled := p.nm.GetWirelessEnabled()
+	flightModeCh, _, err := p.urfkill.WatchFlightMode()
+	if err != nil {
+		return err
+	}
+	wirelessEnabledCh, _, err := p.nm.WatchWirelessEnabled()
+	if err != nil {
+		return err
+	}
+
+	filteredWakeUpCh := make(chan bool)
+	go p.control(wakeupCh, filteredWakeUpCh, flightMode, flightModeCh, wirelessEnabled, wirelessEnabledCh)
+	go p.run(filteredWakeUpCh, doneCh)
 	return nil
 }
 
+func (p *poller) doRequestWakeup(delta time.Duration) (time.Time, string, error) {
+	t := time.Now().Add(delta).Truncate(time.Second)
+	cookie, err := p.powerd.RequestWakeup("ubuntu push client", t)
+	if err == nil {
+		p.log.Debugf("requested wakeup at %s", t)
+	} else {
+		p.log.Errorf("RequestWakeup got %v", err)
+		t = time.Time{}
+		cookie = ""
+	}
+	return t, cookie, err
+}
+
+func (p *poller) control(wakeupCh <-chan bool, filteredWakeUpCh chan<- bool, flightMode bool, flightModeCh <-chan bool, wirelessEnabled bool, wirelessEnabledCh <-chan bool) {
+	dontPoll := flightMode && !wirelessEnabled
+	var t time.Time
+	cookie := ""
+	holdsWakeLock := false
+	for {
+		select {
+		case holdsWakeLock = <-p.holdsWakeLockCh:
+		case <-p.requestWakeupCh:
+			if !t.IsZero() || dontPoll {
+				// earlier wakeup or we shouldn't be polling
+				// => don't request wakeup
+				if dontPoll {
+					p.log.Debugf("skip requesting wakeup")
+				}
+				p.requestedWakeupErrCh <- nil
+				break
+			}
+			var err error
+			t, cookie, err = p.doRequestWakeup(p.times.AlarmInterval)
+			p.requestedWakeupErrCh <- err
+		case b := <-wakeupCh:
+			// seems we get here also on clear wakeup, oh well
+			if !b {
+				panic("WatchWakeups channel produced a false value (??)")
+			}
+			// the channel will produce a true for every
+			// wakeup, not only the one we asked for
+			now := time.Now()
+			if t.IsZero() {
+				p.log.Debugf("got woken up; time is %s", now)
+			} else {
+				p.log.Debugf("got woken up; time is %s (𝛥: %s)", now, now.Sub(t))
+				if !now.Before(t) {
+					t = time.Time{}
+					filteredWakeUpCh <- true
+				}
+			}
+		case flightMode = <-flightModeCh:
+		case wirelessEnabled = <-wirelessEnabledCh:
+		}
+		newDontPoll := flightMode && !wirelessEnabled
+		p.log.Debugf("control: flightMode:%v wirelessEnabled:%v prevDontPoll:%v dontPoll:%v wakeupReq:%v holdsWakeLock:%v", flightMode, wirelessEnabled, dontPoll, newDontPoll, !t.IsZero(), holdsWakeLock)
+		if newDontPoll != dontPoll {
+			if dontPoll = newDontPoll; dontPoll {
+				if !t.IsZero() {
+					err := p.powerd.ClearWakeup(cookie)
+					if err == nil {
+						// cleared
+						t = time.Time{}
+						p.log.Debugf("cleared wakeup")
+					} else {
+						p.log.Errorf("ClearWakeup got %v", err)
+					}
+				}
+			} else {
+				if t.IsZero() && !holdsWakeLock {
+					// reschedule soon
+					t, cookie, _ = p.doRequestWakeup(p.times.NetworkWait / 20)
+				}
+			}
+		}
+	}
+}
+
+func (p *poller) requestWakeup() error {
+	p.requestWakeupCh <- struct{}{}
+	return <-p.requestedWakeupErrCh
+}
+
+func (p *poller) holdsWakeLock(has bool) {
+	p.holdsWakeLockCh <- has
+}
+
 func (p *poller) run(wakeupCh <-chan bool, doneCh <-chan bool) {
 	var lockCookie string
 
@@ -143,15 +282,13 @@
 
 func (p *poller) step(wakeupCh <-chan bool, doneCh <-chan bool, lockCookie string) string {
 
-	t := time.Now().Add(p.times.AlarmInterval).Truncate(time.Second)
-	_, err := p.powerd.RequestWakeup("ubuntu push client", t)
+	err := p.requestWakeup()
 	if err != nil {
-		p.log.Errorf("RequestWakeup got %v", err)
 		// Don't do this too quickly. Pretend we are just skipping one wakeup
 		time.Sleep(p.times.AlarmInterval)
 		return lockCookie
 	}
-	p.log.Debugf("requested wakeup at %s", t)
+	p.holdsWakeLock(false)
 	if lockCookie != "" {
 		if err := p.powerd.ClearWakelock(lockCookie); err != nil {
 			p.log.Errorf("ClearWakelock(%#v) got %v", lockCookie, err)
@@ -160,30 +297,23 @@
 		}
 		lockCookie = ""
 	}
-	for b := range wakeupCh {
-		if !b {
-			panic("WatchWakeups channel produced a false value (??)")
-		}
-		// the channel will produce a true for every
-		// wakeup, not only the one we asked for
-		now := time.Now()
-		p.log.Debugf("got woken up; time is %s (𝛥: %s)", now, now.Sub(t))
-		if !now.Before(t) {
-			break
-		}
-	}
+	<-wakeupCh
 	lockCookie, err = p.powerd.RequestWakelock("ubuntu push client")
 	if err != nil {
 		p.log.Errorf("RequestWakelock got %v", err)
 		return lockCookie
 	}
-	p.log.Debugf("got wakelock cookie of %s", lockCookie)
+	p.holdsWakeLock(true)
+	p.log.Debugf("got wakelock cookie of %s, checking conn state", lockCookie)
 	time.Sleep(p.times.SessionStateSettle)
 	for i := 0; i < 20; i++ {
 		if p.IsConnected() {
+			p.log.Debugf("iter %02d: connected", i)
 			break
 		}
+		p.log.Debugf("iter %02d: not connected, sleeping for %s", i, p.times.NetworkWait/20)
 		time.Sleep(p.times.NetworkWait / 20)
+		p.log.Debugf("iter %02d: slept", i)
 	}
 	if !p.IsConnected() {
 		p.log.Errorf("not connected after %s; giving up", p.times.NetworkWait)
@@ -215,7 +345,9 @@
 		}
 
 		// XXX check whether something was actually done before waiting
+		p.log.Debugf("sleeping for DoneWait %s", p.times.DoneWait)
 		time.Sleep(p.times.DoneWait)
+		p.log.Debugf("slept")
 	}
 
 	return lockCookie

=== modified file 'poller/poller_test.go'
--- poller/poller_test.go	2014-08-21 10:56:12 +0000
+++ poller/poller_test.go	2015-04-02 09:52:21 +0000
@@ -1,5 +1,5 @@
 /*
- Copyright 2014 Canonical Ltd.
+ Copyright 2014-2015 Canonical Ltd.
 
  This program is free software: you can redistribute it and/or modify it
  under the terms of the GNU General Public License version 3, as published
@@ -42,7 +42,7 @@
 	reqWakeCookie string
 	reqWakeErr    error
 	// WatchWakeups
-	watchWakeCh  <-chan bool
+	watchWakeCh  chan bool
 	watchWakeErr error
 	// RequestWakelock
 	reqLockName   string
@@ -63,6 +63,9 @@
 func (m *myD) RequestWakeup(name string, wakeupTime time.Time) (string, error) {
 	m.reqWakeName = name
 	m.reqWakeTime = wakeupTime
+	time.AfterFunc(100*time.Millisecond, func() {
+		m.watchWakeCh <- true
+	})
 	return m.reqWakeCookie, m.reqWakeErr
 }
 func (m *myD) RequestWakelock(name string) (string, error) {
@@ -73,7 +76,10 @@
 	m.clearLockCookie = cookie
 	return m.clearLockErr
 }
-func (m *myD) ClearWakeup(cookie string) error    { panic("clearwakeup called??") }
+func (m *myD) ClearWakeup(cookie string) error {
+	m.watchWakeCh <- false
+	return nil
+}
 func (m *myD) WatchWakeups() (<-chan bool, error) { return m.watchWakeCh, m.watchWakeErr }
 func (m *myD) Poll() error                        { return m.pollErr }
 func (m *myD) WatchDones() (<-chan bool, error)   { return m.watchDonesCh, m.watchDonesErr }
@@ -86,23 +92,27 @@
 
 func (s *PrSuite) TestStep(c *C) {
 	p := &poller{
-		times:        Times{},
-		log:          s.log,
-		powerd:       s.myd,
-		polld:        s.myd,
-		sessionState: s.myd,
+		times:                Times{},
+		log:                  s.log,
+		powerd:               s.myd,
+		polld:                s.myd,
+		sessionState:         s.myd,
+		requestWakeupCh:      make(chan struct{}),
+		requestedWakeupErrCh: make(chan error),
+		holdsWakeLockCh:      make(chan bool),
 	}
 	s.myd.reqLockCookie = "wakelock cookie"
 	s.myd.stateState = session.Running
-	// we'll get the wakeup right away
 	wakeupCh := make(chan bool, 1)
-	wakeupCh <- true
+	s.myd.watchWakeCh = wakeupCh
 	// we won't get the "done" signal in time ;)
 	doneCh := make(chan bool)
 	// and a channel to get the return value from a goroutine
 	ch := make(chan string)
 	// now, run
-	go func() { ch <- p.step(wakeupCh, doneCh, "old cookie") }()
+	filteredWakeUpCh := make(chan bool)
+	go p.control(wakeupCh, filteredWakeUpCh, false, nil, true, nil)
+	go func() { ch <- p.step(filteredWakeUpCh, doneCh, "old cookie") }()
 	select {
 	case s := <-ch:
 		c.Check(s, Equals, "wakelock cookie")
@@ -112,3 +122,53 @@
 	// check we cleared the old cookie
 	c.Check(s.myd.clearLockCookie, Equals, "old cookie")
 }
+
+func (s *PrSuite) TestControl(c *C) {
+	p := &poller{
+		times:                Times{},
+		log:                  s.log,
+		powerd:               s.myd,
+		polld:                s.myd,
+		sessionState:         s.myd,
+		requestWakeupCh:      make(chan struct{}),
+		requestedWakeupErrCh: make(chan error),
+		holdsWakeLockCh:      make(chan bool),
+	}
+	wakeUpCh := make(chan bool)
+	filteredWakeUpCh := make(chan bool)
+	s.myd.watchWakeCh = make(chan bool, 1)
+	flightModeCh := make(chan bool)
+	wirelessModeCh := make(chan bool)
+	go p.control(wakeUpCh, filteredWakeUpCh, false, flightModeCh, true, wirelessModeCh)
+
+	// works
+	err := p.requestWakeup()
+	c.Assert(err, IsNil)
+	c.Check(<-s.myd.watchWakeCh, Equals, true)
+
+	// there's a wakeup already
+	err = p.requestWakeup()
+	c.Assert(err, IsNil)
+	c.Check(s.myd.watchWakeCh, HasLen, 0)
+
+	// wakeup happens
+	wakeUpCh <- true
+	<-filteredWakeUpCh
+
+	// flight mode
+	flightModeCh <- true
+	wirelessModeCh <- false
+	err = p.requestWakeup()
+	c.Assert(err, IsNil)
+	c.Check(s.myd.watchWakeCh, HasLen, 0)
+
+	// wireless on
+	wirelessModeCh <- true
+	c.Check(<-s.myd.watchWakeCh, Equals, true)
+
+	// wireless off
+	wirelessModeCh <- false
+	// pending wakeup was cleared
+	c.Check(<-s.myd.watchWakeCh, Equals, false)
+
+}

=== modified file 'scripts/click-hook'
--- scripts/click-hook	2014-10-15 11:56:43 +0000
+++ scripts/click-hook	2015-04-02 09:52:21 +0000
@@ -53,6 +53,8 @@
 
 def collect_helpers(helpers_data_path, helpers_data_path_tmp, hooks_path):
     helpers_data = {}
+    if not os.path.isdir(hooks_path):
+        return True
     for hook_fname in os.listdir(hooks_path):
         if not hook_fname.endswith(hook_ext):
             continue

=== modified file 'server/tlsconfig.go'
--- server/tlsconfig.go	2014-09-04 12:51:08 +0000
+++ server/tlsconfig.go	2015-04-02 09:52:21 +0000
@@ -48,6 +48,18 @@
 	tlsCfg := &tls.Config{
 		Certificates:           []tls.Certificate{cfg.cert},
 		SessionTicketsDisabled: true,
+		// order from crypto/tls/cipher_suites.go, no RC4
+		CipherSuites: []uint16{
+			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+			tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+			tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+			tls.TLS_RSA_WITH_AES_128_CBC_SHA,
+			tls.TLS_RSA_WITH_AES_256_CBC_SHA,
+		},
+		MinVersion: tls.VersionTLS10,
 	}
 	return tlsCfg
 }