Ubuntu Push Notifications

Merge lp:~pedronis/ubuntu-push/use-sentinel-not-close into lp:ubuntu-push/automatic

  1. use-sentinel-not-close
  2. Merge into automatic
Proposed by Samuele Pedroni
Status: Merged
Approved by: Samuele Pedroni
Approved revision: 128
Merged at revision: 130
Proposed branch: lp:~pedronis/ubuntu-push/use-sentinel-not-close
Merge into: lp:ubuntu-push/automatic
Diff against target: 69 lines (+13/-10)
4 files modified
server/broker/simple/simple.go (+1/-1)
server/broker/testsuite/suite.go (+9/-6)
server/session/session.go (+2/-2)
server/session/session_test.go (+1/-1)
To merge this branch: bzr merge lp:~pedronis/ubuntu-push/use-sentinel-not-close
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+216186@code.launchpad.net

Commit message

use a sentinel of nil instead of close on the session channel to terminate the session, because we use exchanges from two goroutines, so in corner we can race with feedPending and get send on closed channel

Description of the change

use a sentinel of nil instead of close on the session channel to terminate the session, because we use exchanges from two goroutine, so in corner we can race with feedPending and get send on closed channel

To post a comment you must log in.
lp:~pedronis/ubuntu-push/use-sentinel-not-close updated
128. By Samuele Pedroni

merge automatic

Revision history for this message
John Lenton (chipaca) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'server/broker/simple/simple.go'
2--- server/broker/simple/simple.go 2014-04-11 08:47:18 +0000
3+++ server/broker/simple/simple.go 2014-04-16 18:31:22 +0000
4@@ -224,7 +224,7 @@
5 } else { // register
6 prev := b.registry[sess.deviceId]
7 if prev != nil { // kick it
8- close(prev.exchanges)
9+ prev.exchanges <- nil
10 }
11 b.registry[sess.deviceId] = sess
12 sess.registered = true
13
14=== modified file 'server/broker/testsuite/suite.go'
15--- server/broker/testsuite/suite.go 2014-04-11 08:47:18 +0000
16+++ server/broker/testsuite/suite.go 2014-04-16 18:31:22 +0000
17@@ -164,14 +164,17 @@
18 c.Assert(err, IsNil)
19 sess2, err := b.Register(&protocol.ConnectMsg{Type: "connect", DeviceId: "dev-1"})
20 c.Assert(err, IsNil)
21- checkAndFalse := false
22- // previous session got signaled by closing its channel
23+ // previous session got signaled by sending nil on its channel
24+ var sentinel broker.Exchange
25+ got := false
26 select {
27- case _, ok := <-sess1.SessionChannel():
28- checkAndFalse = ok == false
29- default:
30+ case sentinel = <-sess1.SessionChannel():
31+ got = true
32+ case <-time.After(5 * time.Second):
33+ c.Fatal("taking too long to get sentinel")
34 }
35- c.Check(checkAndFalse, Equals, true)
36+ c.Check(got, Equals, true)
37+ c.Check(sentinel, IsNil)
38 c.Assert(s.RevealSession(b, "dev-1"), Equals, sess2)
39 b.Unregister(sess1)
40 // just to make sure the unregister was processed
41
42=== modified file 'server/session/session.go'
43--- server/session/session.go 2014-04-16 12:22:56 +0000
44+++ server/session/session.go 2014-04-16 18:31:22 +0000
45@@ -104,9 +104,9 @@
46 return &broker.ErrAbort{"expected PONG message"}
47 }
48 pingTimerReset()
49- case exchg, ok := <-ch:
50+ case exchg := <-ch:
51 pingTimer.Stop()
52- if !ok {
53+ if exchg == nil {
54 return &broker.ErrAbort{"terminated"}
55 }
56 outMsg, inMsg, err := exchg.Prepare(sess)
57
58=== modified file 'server/session/session_test.go'
59--- server/session/session_test.go 2014-04-16 12:22:56 +0000
60+++ server/session/session_test.go 2014-04-16 18:31:22 +0000
61@@ -360,7 +360,7 @@
62 go func() {
63 errCh <- sessionLoop(tp, sess, cfg5msPingInterval2msExchangeTout, nopTrack)
64 }()
65- close(exchanges)
66+ exchanges <- nil
67 err := <-errCh
68 c.Check(err, DeepEquals, &broker.ErrAbort{"terminated"})
69 }

Subscribers

People subscribed via source and target branches