Merge lp:ubuntu-push/automatic into lp:ubuntu-push/vivid-overlay

Proposed by Jonas G. Drange
Status: Merged
Approved by: Jonas G. Drange
Approved revision: 412
Merged at revision: 152
Proposed branch: lp:ubuntu-push/automatic
Merge into: lp:ubuntu-push/vivid-overlay
Diff against target: 291 lines (+44/-39)
4 files modified
client/client.go (+6/-1)
client/client_test.go (+7/-0)
poller/poller.go (+23/-26)
poller/poller_test.go (+8/-12)
To merge this branch: bzr merge lp:ubuntu-push/automatic
Reviewer Review Type Date Requested Status
Ubuntu Push Hackers Pending
Review via email: mp+274044@code.launchpad.net

Commit message

Fix lp:1469398 by using the connectivity state. Fix case where a failed powerd wakeup request would deadlock step().

Description of the change

Fix lp:1469398 by using the connectivity state. Fix case where a failed powerd wakeup request would deadlock step().

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'client/client.go'
--- client/client.go 2015-04-01 17:43:20 +0000
+++ client/client.go 2015-10-10 13:12:24 +0000
@@ -442,6 +442,11 @@
442 return nil442 return nil
443}443}
444444
445func (client *PushClient) handeConnNotification(conn bool) {
446 client.session.HasConnectivity(conn)
447 client.poller.HasConnectivity(conn)
448}
449
445// doLoop connects events with their handlers450// doLoop connects events with their handlers
446func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, unregisterhandler func(*click.AppId), accountshandler func()) {451func (client *PushClient) doLoop(connhandler func(bool), bcasthandler func(*session.BroadcastNotification) error, ucasthandler func(session.AddressedNotification) error, unregisterhandler func(*click.AppId), accountshandler func()) {
447 for {452 for {
@@ -475,7 +480,7 @@
475480
476// Loop calls doLoop with the "real" handlers481// Loop calls doLoop with the "real" handlers
477func (client *PushClient) Loop() {482func (client *PushClient) Loop() {
478 client.doLoop(client.session.HasConnectivity,483 client.doLoop(client.handeConnNotification,
479 client.handleBroadcastNotification,484 client.handleBroadcastNotification,
480 client.handleUnicastNotification,485 client.handleUnicastNotification,
481 client.handleUnregister,486 client.handleUnregister,
482487
=== modified file 'client/client_test.go'
--- client/client_test.go 2015-04-01 16:02:31 +0000
+++ client/client_test.go 2015-10-10 13:12:24 +0000
@@ -1032,6 +1032,7 @@
1032******************************************************************/1032******************************************************************/
10331033
1034type loopSession struct{ hasConn bool }1034type loopSession struct{ hasConn bool }
1035type loopPoller struct{}
10351036
1036func (s *loopSession) ResetCookie() {}1037func (s *loopSession) ResetCookie() {}
1037func (s *loopSession) State() session.ClientSessionState {1038func (s *loopSession) State() session.ClientSessionState {
@@ -1045,6 +1046,11 @@
1045func (s *loopSession) KeepConnection() error { return nil }1046func (s *loopSession) KeepConnection() error { return nil }
1046func (s *loopSession) StopKeepConnection() {}1047func (s *loopSession) StopKeepConnection() {}
10471048
1049func (p *loopPoller) HasConnectivity(hasConn bool) {}
1050func (p *loopPoller) IsConnected() bool { return false }
1051func (p *loopPoller) Start() error { return nil }
1052func (p *loopPoller) Run() error { return nil }
1053
1048func (cs *clientSuite) TestLoop(c *C) {1054func (cs *clientSuite) TestLoop(c *C) {
1049 cli := NewPushClient(cs.configPath, cs.leveldbPath)1055 cli := NewPushClient(cs.configPath, cs.leveldbPath)
1050 cli.connCh = make(chan bool)1056 cli.connCh = make(chan bool)
@@ -1070,6 +1076,7 @@
1070 c.Assert(cli.session, NotNil)1076 c.Assert(cli.session, NotNil)
1071 cli.session.StopKeepConnection()1077 cli.session.StopKeepConnection()
1072 cli.session = &loopSession{}1078 cli.session = &loopSession{}
1079 cli.poller = &loopPoller{}
10731080
1074 go cli.Loop()1081 go cli.Loop()
10751082
10761083
=== modified file 'poller/poller.go'
--- poller/poller.go 2015-09-28 08:24:51 +0000
+++ poller/poller.go 2015-10-10 13:12:24 +0000
@@ -25,7 +25,6 @@
25 "time"25 "time"
2626
27 "launchpad.net/ubuntu-push/bus"27 "launchpad.net/ubuntu-push/bus"
28 "launchpad.net/ubuntu-push/bus/networkmanager"
29 "launchpad.net/ubuntu-push/bus/polld"28 "launchpad.net/ubuntu-push/bus/polld"
30 "launchpad.net/ubuntu-push/bus/powerd"29 "launchpad.net/ubuntu-push/bus/powerd"
31 "launchpad.net/ubuntu-push/client/session"30 "launchpad.net/ubuntu-push/client/session"
@@ -56,6 +55,7 @@
56 IsConnected() bool55 IsConnected() bool
57 Start() error56 Start() error
58 Run() error57 Run() error
58 HasConnectivity(bool)
59}59}
6060
61type PollerSetup struct {61type PollerSetup struct {
@@ -69,9 +69,9 @@
69 log logger.Logger69 log logger.Logger
70 powerd powerd.Powerd70 powerd powerd.Powerd
71 polld polld.Polld71 polld polld.Polld
72 nm networkmanager.NetworkManager
73 cookie string72 cookie string
74 sessionState stater73 sessionState stater
74 connCh chan bool
75 requestWakeupCh chan struct{}75 requestWakeupCh chan struct{}
76 requestedWakeupErrCh chan error76 requestedWakeupErrCh chan error
77 holdsWakeLockCh chan bool77 holdsWakeLockCh chan bool
@@ -84,6 +84,7 @@
84 powerd: nil,84 powerd: nil,
85 polld: nil,85 polld: nil,
86 sessionState: setup.SessionStateGetter,86 sessionState: setup.SessionStateGetter,
87 connCh: make(chan bool),
87 requestWakeupCh: make(chan struct{}),88 requestWakeupCh: make(chan struct{}),
88 requestedWakeupErrCh: make(chan error),89 requestedWakeupErrCh: make(chan error),
89 holdsWakeLockCh: make(chan bool),90 holdsWakeLockCh: make(chan bool),
@@ -94,6 +95,10 @@
94 return p.sessionState.State() == session.Running95 return p.sessionState.State() == session.Running
95}96}
9697
98func (p *poller) HasConnectivity(hasConn bool) {
99 p.connCh <- hasConn
100}
101
97func (p *poller) Start() error {102func (p *poller) Start() error {
98 if p.log == nil {103 if p.log == nil {
99 return ErrUnconfigured104 return ErrUnconfigured
@@ -104,10 +109,9 @@
104109
105 powerdEndp := bus.SystemBus.Endpoint(powerd.BusAddress, p.log)110 powerdEndp := bus.SystemBus.Endpoint(powerd.BusAddress, p.log)
106 polldEndp := bus.SessionBus.Endpoint(polld.BusAddress, p.log)111 polldEndp := bus.SessionBus.Endpoint(polld.BusAddress, p.log)
107 nmEndp := bus.SystemBus.Endpoint(networkmanager.BusAddress, p.log)
108112
109 var wg sync.WaitGroup113 var wg sync.WaitGroup
110 wg.Add(3)114 wg.Add(2)
111 go func() {115 go func() {
112 n := util.NewAutoRedialer(powerdEndp).Redial()116 n := util.NewAutoRedialer(powerdEndp).Redial()
113 p.log.Debugf("powerd dialed on try %d", n)117 p.log.Debugf("powerd dialed on try %d", n)
@@ -118,16 +122,10 @@
118 p.log.Debugf("polld dialed in on try %d", n)122 p.log.Debugf("polld dialed in on try %d", n)
119 wg.Done()123 wg.Done()
120 }()124 }()
121 go func() {
122 n := util.NewAutoRedialer(nmEndp).Redial()
123 p.log.Debugf("NetworkManager dialed on try %d", n)
124 wg.Done()
125 }()
126 wg.Wait()125 wg.Wait()
127126
128 p.powerd = powerd.New(powerdEndp, p.log)127 p.powerd = powerd.New(powerdEndp, p.log)
129 p.polld = polld.New(polldEndp, p.log)128 p.polld = polld.New(polldEndp, p.log)
130 p.nm = networkmanager.New(nmEndp, p.log)
131129
132 // busy sleep loop to workaround go's timer/sleep130 // busy sleep loop to workaround go's timer/sleep
133 // not accounting for time when the system is suspended131 // not accounting for time when the system is suspended
@@ -150,7 +148,7 @@
150 if p.log == nil {148 if p.log == nil {
151 return ErrUnconfigured149 return ErrUnconfigured
152 }150 }
153 if p.powerd == nil || p.polld == nil || p.nm == nil {151 if p.powerd == nil || p.polld == nil {
154 return ErrNotStarted152 return ErrNotStarted
155 }153 }
156 wakeupCh, err := p.powerd.WatchWakeups()154 wakeupCh, err := p.powerd.WatchWakeups()
@@ -161,13 +159,8 @@
161 if err != nil {159 if err != nil {
162 return err160 return err
163 }161 }
164 nmState := p.nm.GetState()
165 nmStateCh, _, err := p.nm.WatchState()
166 if err != nil {
167 return err
168 }
169 filteredWakeUpCh := make(chan bool)162 filteredWakeUpCh := make(chan bool)
170 go p.control(wakeupCh, filteredWakeUpCh, nmState, nmStateCh)163 go p.control(wakeupCh, filteredWakeUpCh)
171 go p.run(filteredWakeUpCh, doneCh)164 go p.run(filteredWakeUpCh, doneCh)
172 return nil165 return nil
173}166}
@@ -185,10 +178,10 @@
185 return t, cookie, err178 return t, cookie, err
186}179}
187180
188func (p *poller) control(wakeupCh <-chan bool, filteredWakeUpCh chan<- bool, nmState networkmanager.State, nmStateCh <-chan networkmanager.State) {181func (p *poller) control(wakeupCh <-chan bool, filteredWakeUpCh chan<- bool) {
189 connected := nmState == networkmanager.ConnectedGlobal182 // Assume a connection, and poll immediately.
183 connected := true
190 dontPoll := !connected184 dontPoll := !connected
191 p.log.Debugf("nmState: %v, networkmanager.ConnectedGlobal: %v", nmState, networkmanager.ConnectedGlobal)
192 var t time.Time185 var t time.Time
193 cookie := ""186 cookie := ""
194 holdsWakeLock := false187 holdsWakeLock := false
@@ -225,11 +218,12 @@
225 filteredWakeUpCh <- true218 filteredWakeUpCh <- true
226 }219 }
227 }220 }
228 case nmState = <-nmStateCh:221 case state := <-p.connCh:
229 connected = nmState == networkmanager.ConnectedGlobal222 connected = state
223 p.log.Debugf("control: connected:%v", state)
230 }224 }
231 newDontPoll := !connected225 newDontPoll := !connected
232 p.log.Debugf("control: nmState:%v prevDontPoll:%v dontPoll:%v wakeupReq:%v holdsWakeLock:%v", nmState, dontPoll, newDontPoll, !t.IsZero(), holdsWakeLock)226 p.log.Debugf("control: prevDontPoll:%v dontPoll:%v wakeupReq:%v holdsWakeLock:%v", dontPoll, newDontPoll, !t.IsZero(), holdsWakeLock)
233 if newDontPoll != dontPoll {227 if newDontPoll != dontPoll {
234 if dontPoll = newDontPoll; dontPoll {228 if dontPoll = newDontPoll; dontPoll {
235 if !t.IsZero() {229 if !t.IsZero() {
@@ -245,7 +239,12 @@
245 } else {239 } else {
246 if t.IsZero() && !holdsWakeLock {240 if t.IsZero() && !holdsWakeLock {
247 // reschedule soon241 // reschedule soon
248 t, cookie, _ = p.doRequestWakeup(p.times.NetworkWait / 20)242 var err error
243 t, cookie, err = p.doRequestWakeup(p.times.NetworkWait / 20)
244 if err != nil {
245 // Make sure we break a potential deadlock by trying again.
246 filteredWakeUpCh <- true
247 }
249 }248 }
250 }249 }
251 }250 }
@@ -281,8 +280,6 @@
281 if lockCookie != "" {280 if lockCookie != "" {
282 if err := p.powerd.ClearWakelock(lockCookie); err != nil {281 if err := p.powerd.ClearWakelock(lockCookie); err != nil {
283 p.log.Errorf("ClearWakelock(%#v) got %v", lockCookie, err)282 p.log.Errorf("ClearWakelock(%#v) got %v", lockCookie, err)
284 } else {
285 p.log.Debugf("cleared wakelock cookie %s.", lockCookie)
286 }283 }
287 lockCookie = ""284 lockCookie = ""
288 }285 }
289286
=== modified file 'poller/poller_test.go'
--- poller/poller_test.go 2015-09-28 08:24:51 +0000
+++ poller/poller_test.go 2015-10-10 13:12:24 +0000
@@ -21,7 +21,6 @@
2121
22 . "launchpad.net/gocheck"22 . "launchpad.net/gocheck"
2323
24 "launchpad.net/ubuntu-push/bus/networkmanager"
25 "launchpad.net/ubuntu-push/client/session"24 "launchpad.net/ubuntu-push/client/session"
26 helpers "launchpad.net/ubuntu-push/testing"25 helpers "launchpad.net/ubuntu-push/testing"
27)26)
@@ -91,11 +90,6 @@
91 s.myd = &myD{}90 s.myd = &myD{}
92}91}
9392
94const (
95 connectedGlobal = networkmanager.ConnectedGlobal
96 disconnectedGlobal = networkmanager.Disconnected
97)
98
99func (s *PrSuite) TestStep(c *C) {93func (s *PrSuite) TestStep(c *C) {
100 p := &poller{94 p := &poller{
101 times: Times{},95 times: Times{},
@@ -106,6 +100,7 @@
106 requestWakeupCh: make(chan struct{}),100 requestWakeupCh: make(chan struct{}),
107 requestedWakeupErrCh: make(chan error),101 requestedWakeupErrCh: make(chan error),
108 holdsWakeLockCh: make(chan bool),102 holdsWakeLockCh: make(chan bool),
103 connCh: make(chan bool),
109 }104 }
110 s.myd.reqLockCookie = "wakelock cookie"105 s.myd.reqLockCookie = "wakelock cookie"
111 s.myd.stateState = session.Running106 s.myd.stateState = session.Running
@@ -117,7 +112,7 @@
117 ch := make(chan string)112 ch := make(chan string)
118 // now, run113 // now, run
119 filteredWakeUpCh := make(chan bool)114 filteredWakeUpCh := make(chan bool)
120 go p.control(wakeupCh, filteredWakeUpCh, connectedGlobal, nil)115 go p.control(wakeupCh, filteredWakeUpCh)
121 go func() { ch <- p.step(filteredWakeUpCh, doneCh, "old cookie") }()116 go func() { ch <- p.step(filteredWakeUpCh, doneCh, "old cookie") }()
122 select {117 select {
123 case s := <-ch:118 case s := <-ch:
@@ -139,14 +134,15 @@
139 requestWakeupCh: make(chan struct{}),134 requestWakeupCh: make(chan struct{}),
140 requestedWakeupErrCh: make(chan error),135 requestedWakeupErrCh: make(chan error),
141 holdsWakeLockCh: make(chan bool),136 holdsWakeLockCh: make(chan bool),
137 connCh: make(chan bool),
142 }138 }
143 wakeUpCh := make(chan bool)139 wakeUpCh := make(chan bool)
144 filteredWakeUpCh := make(chan bool)140 filteredWakeUpCh := make(chan bool)
145 s.myd.watchWakeCh = make(chan bool, 1)141 s.myd.watchWakeCh = make(chan bool, 1)
146 nmStateCh := make(chan networkmanager.State)142 go p.control(wakeUpCh, filteredWakeUpCh)
147 go p.control(wakeUpCh, filteredWakeUpCh, connectedGlobal, nmStateCh)
148143
149 // works144 // works
145 p.HasConnectivity(true)
150 err := p.requestWakeup()146 err := p.requestWakeup()
151 c.Assert(err, IsNil)147 c.Assert(err, IsNil)
152 c.Check(<-s.myd.watchWakeCh, Equals, true)148 c.Check(<-s.myd.watchWakeCh, Equals, true)
@@ -160,17 +156,17 @@
160 wakeUpCh <- true156 wakeUpCh <- true
161 <-filteredWakeUpCh157 <-filteredWakeUpCh
162158
163 nmStateCh <- disconnectedGlobal159 p.HasConnectivity(false)
164 err = p.requestWakeup()160 err = p.requestWakeup()
165 c.Assert(err, IsNil)161 c.Assert(err, IsNil)
166 c.Check(s.myd.watchWakeCh, HasLen, 0)162 c.Check(s.myd.watchWakeCh, HasLen, 0)
167163
168 // connected164 // connected
169 nmStateCh <- connectedGlobal165 p.HasConnectivity(true)
170 c.Check(<-s.myd.watchWakeCh, Equals, true)166 c.Check(<-s.myd.watchWakeCh, Equals, true)
171167
172 // disconnected168 // disconnected
173 nmStateCh <- disconnectedGlobal169 p.HasConnectivity(false)
174 // pending wakeup was cleared170 // pending wakeup was cleared
175 c.Check(<-s.myd.watchWakeCh, Equals, false)171 c.Check(<-s.myd.watchWakeCh, Equals, false)
176172

Subscribers

People subscribed via source and target branches

to all changes: