From 373dbd355e5cb4b1f763647464a98d2e692636d0 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 21:56:22 -0700 Subject: [PATCH] Better handling of subscribers to avoid a slice panic --- server/events.go | 29 ++++++++++++++++++++++++++--- server/server.go | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/server/events.go b/server/events.go index 425de25..71da55b 100644 --- a/server/events.go +++ b/server/events.go @@ -32,6 +32,9 @@ type EventBus struct { // Returns the server's emitter instance. func (s *Server) Events() *EventBus { + s.emitterLock.Lock() + defer s.emitterLock.Unlock() + if s.emitter == nil { s.emitter = &EventBus{ subscribers: map[string][]chan Event{}, @@ -85,11 +88,27 @@ func (e *EventBus) Subscribe(topic string, ch chan Event) { e.Lock() defer e.Unlock() - if p, ok := e.subscribers[topic]; ok { - e.subscribers[topic] = append(p, ch) - } else { + p, ok := e.subscribers[topic] + + // If there is nothing currently subscribed to this topic just go ahead and create + // the item and then return. + if !ok { e.subscribers[topic] = append([]chan Event{}, ch) + return } + + // If this topic is already setup, first iterate over the event channels currently in there + // and confirm there is not a match. If there _is_ a match do nothing since that means this + // channel is already being tracked. This avoids registering two identical handlers for the + // same topic, and means the Unsubscribe function can safely assume there will only be a + // single match for an event. + for i := range e.subscribers[topic] { + if ch == e.subscribers[topic][i] { + return + } + } + + e.subscribers[topic] = append(p, ch) } // Unsubscribe a channel from a topic. @@ -101,6 +120,10 @@ func (e *EventBus) Unsubscribe(topic string, ch chan Event) { for i := range e.subscribers[topic] { if ch == e.subscribers[topic][i] { e.subscribers[topic] = append(e.subscribers[topic][:i], e.subscribers[topic][i+1:]...) + // Subscribe enforces a unique event channel for the topic, so we can safely exit + // this loop once matched since there should not be any additional matches after + // this point. + break } } } diff --git a/server/server.go b/server/server.go index e07a340..7530f36 100644 --- a/server/server.go +++ b/server/server.go @@ -19,6 +19,7 @@ type Server struct { // Internal mutex used to block actions that need to occur sequentially, such as // writing the configuration to the disk. sync.RWMutex + emitterLock sync.Mutex // Maintains the configuration for the server. This is the data that gets returned by the Panel // such as build settings and container images.