event: address review issues (multiple commits)

event: address Feed review issues

event: clarify role of NewSubscription function

event: more Feed review fixes

* take sendLock after dropping f.mu
* add constant for number of special cases

event: fix subscribing/unsubscribing while Send is blocked

1 files changed, 20 insertions, 11 deletions

diff --git a/event/feed.go b/event/feed.go
index bd8e26321..4568304df 100644
--- a/event/feed.go
+++ b/event/feed.go
@@ -33,7 +33,9 @@ var errBadChannel = errors.New("event: Subscribe argument does not have sendable
 //
 // The zero value is ready to use.
 type Feed struct {
-	sendLock  chan struct{}    // one-element buffer, empty when held
+	// sendLock has a one-element buffer and is empty when held.
+	// It protects sendCases.
+	sendLock  chan struct{}
 	removeSub chan interface{} // interrupts Send
 	sendCases caseList         // the active set of select cases used by Send
 
@@ -44,6 +46,10 @@ type Feed struct {
 	closed bool
 }
 
+// This is the index of the first actual subscription channel in sendCases.
+// sendCases[0] is a SelectRecv case for the removeSub channel.
+const firstSubSendCase = 1
+
 type feedTypeError struct {
 	got, want reflect.Type
 	op        string
@@ -67,6 +73,7 @@ func (f *Feed) init() {
 // until the subscription is canceled. All channels added must have the same element type.
 //
 // The channel should have ample buffer space to avoid blocking other subscribers.
+// Slow subscribers are not dropped.
 func (f *Feed) Subscribe(channel interface{}) Subscription {
 	chanval := reflect.ValueOf(channel)
 	chantyp := chanval.Type()
@@ -125,13 +132,14 @@ func (f *Feed) remove(sub *feedSub) {
 func (f *Feed) Send(value interface{}) (nsent int) {
 	f.mu.Lock()
 	f.init()
+	f.mu.Unlock()
+
 	<-f.sendLock
-	// Add new subscriptions from the inbox, then clear it.
+
+	// Add new cases from the inbox after taking the send lock.
+	f.mu.Lock()
 	f.sendCases = append(f.sendCases, f.inbox...)
-	for i := range f.inbox {
-		f.inbox[i] = reflect.SelectCase{}
-	}
-	f.inbox = f.inbox[:0]
+	f.inbox = nil
 	f.mu.Unlock()
 
 	// Set the sent value on all channels.
@@ -140,7 +148,7 @@ func (f *Feed) Send(value interface{}) (nsent int) {
 		f.sendLock <- struct{}{}
 		panic(feedTypeError{op: "Send", got: rvalue.Type(), want: f.etype})
 	}
-	for i := 1; i < len(f.sendCases); i++ {
+	for i := firstSubSendCase; i < len(f.sendCases); i++ {
 		f.sendCases[i].Send = rvalue
 	}
 
@@ -150,13 +158,14 @@ func (f *Feed) Send(value interface{}) (nsent int) {
 		// Fast path: try sending without blocking before adding to the select set.
 		// This should usually succeed if subscribers are fast enough and have free
 		// buffer space.
-		for i := 1; i < len(cases); i++ {
+		for i := firstSubSendCase; i < len(cases); i++ {
 			if cases[i].Chan.TrySend(rvalue) {
-				cases = cases.deactivate(i)
 				nsent++
+				cases = cases.deactivate(i)
+				i--
 			}
 		}
-		if len(cases) == 1 {
+		if len(cases) == firstSubSendCase {
 			break
 		}
 		// Select on all the receivers, waiting for them to unblock.
@@ -174,7 +183,7 @@ func (f *Feed) Send(value interface{}) (nsent int) {
 	}
 
 	// Forget about the sent value and hand off the send lock.
-	for i := 1; i < len(f.sendCases); i++ {
+	for i := firstSubSendCase; i < len(f.sendCases); i++ {
 		f.sendCases[i].Send = reflect.Value{}
 	}
 	f.sendLock <- struct{}{}