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

[Samuele Pedroni]

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

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

[lp:~pedronis/ubuntu-push/use-sentinel-not-close]

128. By Samuele Pedroni on 2014-04-16

merge automatic

[John Lenton (chipaca)]

review: Approve

=== modified file 'server/broker/simple/simple.go'
--- server/broker/simple/simple.go	2014-04-11 08:47:18 +0000
+++ server/broker/simple/simple.go	2014-04-16 18:31:22 +0000
@@ -224,7 +224,7 @@
 			} else { // register
 				prev := b.registry[sess.deviceId]
 				if prev != nil { // kick it
-					close(prev.exchanges)
+					prev.exchanges <- nil
 				}
 				b.registry[sess.deviceId] = sess
 				sess.registered = true
 
=== modified file 'server/broker/testsuite/suite.go'
--- server/broker/testsuite/suite.go	2014-04-11 08:47:18 +0000
+++ server/broker/testsuite/suite.go	2014-04-16 18:31:22 +0000
@@ -164,14 +164,17 @@
 	c.Assert(err, IsNil)
 	sess2, err := b.Register(&protocol.ConnectMsg{Type: "connect", DeviceId: "dev-1"})
 	c.Assert(err, IsNil)
-	checkAndFalse := false
-	// previous session got signaled by closing its channel
+	// previous session got signaled by sending nil on its channel
+	var sentinel broker.Exchange
+	got := false
 	select {
-	case _, ok := <-sess1.SessionChannel():
-		checkAndFalse = ok == false
-	default:
+	case sentinel = <-sess1.SessionChannel():
+		got = true
+	case <-time.After(5 * time.Second):
+		c.Fatal("taking too long to get sentinel")
 	}
-	c.Check(checkAndFalse, Equals, true)
+	c.Check(got, Equals, true)
+	c.Check(sentinel, IsNil)
 	c.Assert(s.RevealSession(b, "dev-1"), Equals, sess2)
 	b.Unregister(sess1)
 	// just to make sure the unregister was processed
 
=== modified file 'server/session/session.go'
--- server/session/session.go	2014-04-16 12:22:56 +0000
+++ server/session/session.go	2014-04-16 18:31:22 +0000
@@ -104,9 +104,9 @@
 				return &broker.ErrAbort{"expected PONG message"}
 			}
 			pingTimerReset()
-		case exchg, ok := <-ch:
+		case exchg := <-ch:
 			pingTimer.Stop()
-			if !ok {
+			if exchg == nil {
 				return &broker.ErrAbort{"terminated"}
 			}
 			outMsg, inMsg, err := exchg.Prepare(sess)
 
=== modified file 'server/session/session_test.go'
--- server/session/session_test.go	2014-04-16 12:22:56 +0000
+++ server/session/session_test.go	2014-04-16 18:31:22 +0000
@@ -360,7 +360,7 @@
 	go func() {
 		errCh <- sessionLoop(tp, sess, cfg5msPingInterval2msExchangeTout, nopTrack)
 	}()
-	close(exchanges)
+	exchanges <- nil
 	err := <-errCh
 	c.Check(err, DeepEquals, &broker.ErrAbort{"terminated"})
 }