Merge pull request #206 from mum4k/205-deadlock

Release widget's mutex before activating external callback.

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Termdash could deadlock when a `Button` or a `TextInput` was configured to
+  call the `Container.Update` method.
+
 ## [0.9.0] - 28-Apr-2019
 
 ### Added

widgets/button/button.go

@@ -154,11 +154,8 @@ func (b *Button) Draw(cvs *canvas.Canvas, meta *widgetapi.Meta) error {
 	)
 }
 
-// Keyboard processes keyboard events, acts as a button press on the configured
-// Key.
-//
-// Implements widgetapi.Widget.Keyboard.
-func (b *Button) Keyboard(k *terminalapi.Keyboard) error {
+// activated asserts whether the keyboard event activated the button.
+func (b *Button) keyActivated(k *terminalapi.Keyboard) bool {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
@@ -166,16 +163,27 @@ func (b *Button) Keyboard(k *terminalapi.Keyboard) error {
 		b.state = button.Down
 		now := time.Now().UTC()
 		b.keyTriggerTime = &now
+		return true
+	}
+	return false
+}
+
+// Keyboard processes keyboard events, acts as a button press on the configured
+// Key.
+//
+// Implements widgetapi.Widget.Keyboard.
+func (b *Button) Keyboard(k *terminalapi.Keyboard) error {
+	if b.keyActivated(k) {
+		// Mutex must be released when calling the callback.
+		// Users might call container methods from the callback like the
+		// Container.Update, see #205.
 		return b.callback()
 	}
 	return nil
 }
 
-// Mouse processes mouse events, acts as a button press if both the press and
-// the release happen inside the button.
-//
-// Implements widgetapi.Widget.Mouse.
-func (b *Button) Mouse(m *terminalapi.Mouse) error {
+// mouseActivated asserts whether the mouse event activated the button.
+func (b *Button) mouseActivated(m *terminalapi.Mouse) bool {
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
@@ -183,7 +191,18 @@ func (b *Button) Mouse(m *terminalapi.Mouse) error {
 	b.state = state
 	b.keyTriggerTime = nil
 
-	if clicked {
+	return clicked
+}
+
+// Mouse processes mouse events, acts as a button press if both the press and
+// the release happen inside the button.
+//
+// Implements widgetapi.Widget.Mouse.
+func (b *Button) Mouse(m *terminalapi.Mouse) error {
+	if b.mouseActivated(m) {
+		// Mutex must be released when calling the callback.
+		// Users might call container methods from the callback like the
+		// Container.Update, see #205.
 		return b.callback()
 	}
 	return nil

widgets/textinput/textinput.go

@@ -220,9 +220,11 @@ func (ti *TextInput) Draw(cvs *canvas.Canvas, meta *widgetapi.Meta) error {
 	return nil
 }
 
-// Keyboard processes keyboard events.
+// keyboard processes keyboard events.
+// Returns a bool indicating if the content was submitted and the text in the
+// field at submission time.
 // Implements widgetapi.Widget.Keyboard.
-func (ti *TextInput) Keyboard(k *terminalapi.Keyboard) error {
+func (ti *TextInput) keyboard(k *terminalapi.Keyboard) (bool, string) {
 	ti.mu.Lock()
 	defer ti.mu.Unlock()
 
@@ -251,21 +253,33 @@ func (ti *TextInput) Keyboard(k *terminalapi.Keyboard) error {
 			ti.editor.reset()
 		}
 		if ti.opts.onSubmit != nil {
-			return ti.opts.onSubmit(text)
+			return true, text
 		}
 
 	default:
 		if err := wrap.ValidText(string(k.Key)); err != nil {
 			// Ignore unsupported runes.
-			return nil
+			return false, ""
 		}
 		if ti.opts.filter != nil && !ti.opts.filter(rune(k.Key)) {
 			// Ignore filtered runes.
-			return nil
+			return false, ""
 		}
 		ti.editor.insert(rune(k.Key))
 	}
 
+	return false, ""
+}
+
+// Keyboard processes keyboard events.
+// Implements widgetapi.Widget.Keyboard.
+func (ti *TextInput) Keyboard(k *terminalapi.Keyboard) error {
+	if submitted, text := ti.keyboard(k); submitted {
+		// Mutex must be released when calling the callback.
+		// Users might call container methods from the callback like the
+		// Container.Update, see #205.
+		return ti.opts.onSubmit(text)
+	}
 	return nil
 }