From 6980f88810aabb80a5247f6fb3d27477ac3bef6b Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 13 May 2019 22:28:41 -0400 Subject: [PATCH 1/2] Release widget's mutex before activating external callback. Fixes #205. --- widgets/button/button.go | 41 +++++++++++++++++++++++++--------- widgets/textinput/textinput.go | 24 +++++++++++++++----- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/widgets/button/button.go b/widgets/button/button.go index 0a7544e..beaf5ca 100644 --- a/widgets/button/button.go +++ b/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 diff --git a/widgets/textinput/textinput.go b/widgets/textinput/textinput.go index cc2de60..5b0b05c 100644 --- a/widgets/textinput/textinput.go +++ b/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 } From ead318993bd944dcb1d7e212a6b51c4000912261 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 13 May 2019 22:44:00 -0400 Subject: [PATCH 2/2] Updating CHANGELOG. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fdfa5d..4f7fd65 100644 --- a/CHANGELOG.md +++ b/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