From dbd182b6dabe3f7faa0f1303ed4388acaade3f45 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Fri, 14 Oct 2022 21:00:59 -0500 Subject: [PATCH] widgets: lock access to WidgetWatchers internal map The WidgetWatchers struct is designed to be embedded into custom widgets in order to satisfy the WidgetWatcher interface. The implementation uses an internal map for storing EventHandlers, but does not lock access to the access of the map within each method. A race condition can occur if Watch, Unwatch, or any event is Posted from separate goroutines. Lock access to the internal map. When posting an event, create a deep copy of the map for iterating. This prevents potential deadlocks from a widget calling Unwatch while handling an event. --- views/widget.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/views/widget.go b/views/widget.go index 626c43ab..279e452a 100644 --- a/views/widget.go +++ b/views/widget.go @@ -15,6 +15,8 @@ package views import ( + "sync" + "github.com/gdamore/tcell/v2" ) @@ -81,16 +83,21 @@ func (wev *widgetEvent) SetWidget(widget Widget) { wev.widget = widget } -// WidgetWatchers provides a common implementation for base Widget -// Watch and Unwatch interfaces, suitable for embedding in more concrete -// widget implementations. +// WidgetWatchers provides a common implementation for base Widget Watch and +// Unwatch interfaces, suitable for embedding in more concrete widget +// implementations. This implementation is thread-safe, meaning an embedder can +// expect safety when calling WidgetWatcher methods from separate goroutines. +// In general, the views package does not support thread safety. type WidgetWatchers struct { + sync.Mutex watchers map[tcell.EventHandler]struct{} } // Watch monitors this WidgetWatcher, causing the handler to be fired // with EventWidget as they are occur on the watched Widget. func (ww *WidgetWatchers) Watch(handler tcell.EventHandler) { + ww.Lock() + defer ww.Unlock() if ww.watchers == nil { ww.watchers = make(map[tcell.EventHandler]struct{}) } @@ -100,6 +107,8 @@ func (ww *WidgetWatchers) Watch(handler tcell.EventHandler) { // Unwatch stops monitoring this WidgetWatcher. The handler will no longer // be fired for Widget events. func (ww *WidgetWatchers) Unwatch(handler tcell.EventHandler) { + ww.Lock() + defer ww.Unlock() if ww.watchers != nil { delete(ww.watchers, handler) } @@ -108,7 +117,13 @@ func (ww *WidgetWatchers) Unwatch(handler tcell.EventHandler) { // PostEvent delivers the EventWidget to all registered watchers. It is // to be called by the Widget implementation. func (ww *WidgetWatchers) PostEvent(wev EventWidget) { - for watcher := range ww.watchers { + ww.Lock() + watcherCopy := make(map[tcell.EventHandler]struct{}, len(ww.watchers)) + for k := range ww.watchers { + watcherCopy[k] = struct{}{} + } + ww.Unlock() + for watcher := range watcherCopy { // Deliver events to all listeners, ignoring return value. watcher.HandleEvent(wev) }