Skip to content

Commit

Permalink
widgets: lock access to WidgetWatchers internal map
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rockorager authored and gdamore committed Oct 17, 2022
1 parent b9dc8a6 commit dbd182b
Showing 1 changed file with 19 additions and 4 deletions.
23 changes: 19 additions & 4 deletions views/widget.go
Expand Up @@ -15,6 +15,8 @@
package views

import (
"sync"

"github.com/gdamore/tcell/v2"
)

Expand Down Expand Up @@ -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{})
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down

0 comments on commit dbd182b

Please sign in to comment.