From 313fed77b2dd57f47b5e1e81938d54f6a1e57118 Mon Sep 17 00:00:00 2001
From: Marc CB <marc.cbrooks@gmail.com>
Date: Tue, 9 Oct 2018 00:02:02 -0400
Subject: [PATCH 1/2] make primitive updates thread-safe using channels

---
 application.go | 140 +++++++++++++++++++++++++++++++------------------
 1 file changed, 89 insertions(+), 51 deletions(-)

diff --git a/application.go b/application.go
index 146954a..29cd4ed 100644
--- a/application.go
+++ b/application.go
@@ -46,11 +46,20 @@ type Application struct {
 
 	// Halts the event loop during suspended mode.
 	suspendMutex sync.Mutex
+
+	// Used to send screen events from separate goroutine to main event loop
+	events chan tcell.Event
+
+	// Used to send primitive updates from separate goroutines to the main event loop
+	updates chan func()
 }
 
 // NewApplication creates and returns a new application.
 func NewApplication() *Application {
-	return &Application{}
+	return &Application{
+		events:  make(chan tcell.Event, 100),
+		updates: make(chan func(), 100),
+	}
 }
 
 // SetInputCapture sets a function which captures all key events before they are
@@ -136,61 +145,78 @@ func (a *Application) Run() error {
 	a.Unlock()
 	a.Draw()
 
-	// Start event loop.
-	for {
-		// Do not poll events during suspend mode
-		a.suspendMutex.Lock()
-		a.RLock()
-		screen := a.screen
-		a.RUnlock()
-		if screen == nil {
-			a.suspendMutex.Unlock()
-			break
-		}
-
-		// Wait for next event.
-		event := a.screen.PollEvent()
-		a.suspendMutex.Unlock()
-		if event == nil {
-			// The screen was finalized. Exit the loop.
-			break
-		}
-
-		switch event := event.(type) {
-		case *tcell.EventKey:
-			a.RLock()
-			p := a.focus
-			a.RUnlock()
-
-			// Intercept keys.
-			if a.inputCapture != nil {
-				event = a.inputCapture(event)
-				if event == nil {
-					break // Don't forward event.
-				}
-			}
-
-			// Ctrl-C closes the application.
-			if event.Key() == tcell.KeyCtrlC {
-				a.Stop()
-			}
-
-			// Pass other key events to the currently focused primitive.
-			if p != nil {
-				if handler := p.InputHandler(); handler != nil {
-					handler(event, func(p Primitive) {
-						a.SetFocus(p)
-					})
-					a.Draw()
-				}
-			}
-		case *tcell.EventResize:
+	// Separate loop to wait for screen events
+	go func() {
+		for {
+			// Do not poll events during suspend mode
+			a.suspendMutex.Lock()
 			a.RLock()
 			screen := a.screen
 			a.RUnlock()
-			screen.Clear()
+			if screen == nil {
+				a.suspendMutex.Unlock()
+				// send signal to stop main event loop
+				a.QueueEvent(nil)
+				break
+			}
+
+			// Wait for next event.
+			a.QueueEvent(screen.PollEvent())
+			a.suspendMutex.Unlock()
+		}
+	}()
+
+	// Start event loop.
+loop:
+	for {
+		select {
+		case event := <-a.events:
+			if event == nil {
+				// The screen was finalized. Exit the loop.
+				break loop
+			}
+
+			switch event := event.(type) {
+			case *tcell.EventKey:
+				a.RLock()
+				p := a.focus
+				a.RUnlock()
+
+				// Intercept keys.
+				if a.inputCapture != nil {
+					event = a.inputCapture(event)
+					if event == nil {
+						break loop // Don't forward event.
+					}
+				}
+
+				// Ctrl-C closes the application.
+				if event.Key() == tcell.KeyCtrlC {
+					a.Stop()
+				}
+
+				// Pass other key events to the currently focused primitive.
+				if p != nil {
+					if handler := p.InputHandler(); handler != nil {
+						handler(event, func(p Primitive) {
+							a.SetFocus(p)
+						})
+						a.Draw()
+					}
+				}
+			case *tcell.EventResize:
+				a.RLock()
+				screen := a.screen
+				a.RUnlock()
+				screen.Clear()
+				a.Draw()
+			}
+
+		case updater := <-a.updates:
+			updater()
 			a.Draw()
 		}
+
 	}
 
 	return nil
@@ -404,3 +430,15 @@ func (a *Application) GetFocus() Primitive {
 	defer a.RUnlock()
 	return a.focus
 }
+
+// QueueUpdate is used to synchronize changes to primitives by carrying an update function from separate goroutine to the Application event loop via channel
+func (a *Application) QueueUpdate(f func()) *Application {
+	a.updates <- f
+	return a
+}
+
+// QueueEvent takes an Event instance and sends it to the Application event loop via channel
+func (a *Application) QueueEvent(e tcell.Event) *Application {
+	a.events <- e
+	return a
+}

From c22d5570bec925a8e6a7105fadf06bd39d2913fb Mon Sep 17 00:00:00 2001
From: Oliver <480930+rivo@users.noreply.github.com>
Date: Sun, 28 Oct 2018 13:42:49 +0100
Subject: [PATCH 2/2] Bugfixes/improvements to PR #172.

---
 README.md                      |   2 +
 application.go                 | 167 +++++++++++++++++++++------------
 demos/presentation/textview.go |   9 +-
 doc.go                         |  21 +++++
 textview.go                    |  42 ++++++++-
 5 files changed, 173 insertions(+), 68 deletions(-)

diff --git a/README.md b/README.md
index 99e89b6..5a182b6 100644
--- a/README.md
+++ b/README.md
@@ -65,6 +65,8 @@ Add your issue here on GitHub. Feel free to get in touch if you have any questio
 
 (There are no corresponding tags in the project. I only keep such a history in this README.)
 
+- v0.19 (2018-10-28)
+  - Added `QueueUpdate()` and `QueueEvent()` to `Application` to help with modifications to primitives from goroutines.
 - v0.18 (2018-10-18)
   - `InputField` elements can now be navigated freely.
 - v0.17 (2018-06-20)
diff --git a/application.go b/application.go
index 29cd4ed..4dcae09 100644
--- a/application.go
+++ b/application.go
@@ -1,13 +1,14 @@
 package tview
 
 import (
-	"fmt"
-	"os"
 	"sync"
 
 	"github.com/gdamore/tcell"
 )
 
+// The size of the event/update/redraw channels.
+const queueSize = 100
+
 // Application represents the top node of an application.
 //
 // It is not strictly required to use this class as none of the other classes
@@ -19,7 +20,8 @@ type Application struct {
 	// The application's screen.
 	screen tcell.Screen
 
-	// Indicates whether the application's screen is currently active.
+	// Indicates whether the application's screen is currently active. This is
+	// false during suspended mode.
 	running bool
 
 	// The primitive which currently has the keyboard focus.
@@ -44,21 +46,26 @@ type Application struct {
 	// was drawn.
 	afterDraw func(screen tcell.Screen)
 
-	// Halts the event loop during suspended mode.
-	suspendMutex sync.Mutex
-
 	// Used to send screen events from separate goroutine to main event loop
 	events chan tcell.Event
 
-	// Used to send primitive updates from separate goroutines to the main event loop
+	// Functions queued from goroutines, used to serialize updates to primitives.
 	updates chan func()
+
+	// Redraw requests.
+	redraw chan struct{}
+
+	// A channel which signals the end of the suspended mode.
+	suspendToken chan struct{}
 }
 
 // NewApplication creates and returns a new application.
 func NewApplication() *Application {
 	return &Application{
-		events:  make(chan tcell.Event, 100),
-		updates: make(chan func(), 100),
+		events:       make(chan tcell.Event, queueSize),
+		updates:      make(chan func(), queueSize),
+		redraw:       make(chan struct{}, queueSize),
+		suspendToken: make(chan struct{}, 1),
 	}
 }
 
@@ -143,50 +150,70 @@ func (a *Application) Run() error {
 
 	// Draw the screen for the first time.
 	a.Unlock()
-	a.Draw()
+	a.draw()
 
-	// Separate loop to wait for screen events
+	// Separate loop to wait for screen events.
+	var wg sync.WaitGroup
+	wg.Add(1)
+	a.suspendToken <- struct{}{} // We need this to get started.
 	go func() {
-		for {
-			// Do not poll events during suspend mode
-			a.suspendMutex.Lock()
-			a.RLock()
-			screen := a.screen
-			a.RUnlock()
-			if screen == nil {
-				a.suspendMutex.Unlock()
-				// send signal to stop main event loop
-				a.QueueEvent(nil)
+		defer wg.Done()
+		for range a.suspendToken {
+			for {
+				a.RLock()
+				screen := a.screen
+				a.RUnlock()
+				if screen == nil {
+					// We have no screen. We might need to stop.
+					break
+				}
+
+				// Wait for next event and queue it.
+				event := screen.PollEvent()
+				if event != nil {
+					// Regular event. Queue.
+					a.QueueEvent(event)
+					continue
+				}
+
+				// A screen was finalized (event is nil).
+				a.RLock()
+				running := a.running
+				a.RUnlock()
+				if running {
+					// The application was stopped. End the event loop.
+					a.QueueEvent(nil)
+					return
+				}
+
+				// We're in suspended mode (running is false). Pause and wait for new
+				// token.
 				break
 			}
-
-			// Wait for next event.
-			a.QueueEvent(screen.PollEvent())
-			a.suspendMutex.Unlock()
 		}
 	}()
 
 	// Start event loop.
-loop:
+EventLoop:
 	for {
 		select {
 		case event := <-a.events:
 			if event == nil {
-				// The screen was finalized. Exit the loop.
-				break loop
+				break EventLoop
 			}
 
 			switch event := event.(type) {
 			case *tcell.EventKey:
 				a.RLock()
 				p := a.focus
+				inputCapture := a.inputCapture
 				a.RUnlock()
 
 				// Intercept keys.
-				if a.inputCapture != nil {
-					event = a.inputCapture(event)
+				if inputCapture != nil {
+					event = inputCapture(event)
 					if event == nil {
-						break loop // Don't forward event.
+						break EventLoop // Don't forward event.
 					}
 				}
 
@@ -201,7 +228,7 @@ loop:
 						handler(event, func(p Primitive) {
 							a.SetFocus(p)
 						})
-						a.Draw()
+						a.draw()
 					}
 				}
 			case *tcell.EventResize:
@@ -209,16 +236,23 @@ loop:
 				screen := a.screen
 				a.RUnlock()
 				screen.Clear()
-				a.Draw()
+				a.draw()
 			}
 
+		// If we have updates, now is the time to execute them.
 		case updater := <-a.updates:
 			updater()
-			a.Draw()
-		}
 
+			// If a redraw is requested, do it now.
+		case <-a.redraw:
+			a.draw()
+		}
 	}
 
+	a.running = false
+	close(a.suspendToken)
+	wg.Wait()
+
 	return nil
 }
 
@@ -226,12 +260,13 @@ loop:
 func (a *Application) Stop() {
 	a.Lock()
 	defer a.Unlock()
-	if a.screen == nil {
+	screen := a.screen
+	if screen == nil {
 		return
 	}
-	a.screen.Fini()
 	a.screen = nil
-	a.running = false
+	screen.Fini()
+	// a.running is still true, the main loop will clean up.
 }
 
 // Suspend temporarily suspends the application by exiting terminal UI mode and
@@ -242,32 +277,26 @@ func (a *Application) Stop() {
 // was called. If false is returned, the application was already suspended,
 // terminal UI mode was not exited, and "f" was not called.
 func (a *Application) Suspend(f func()) bool {
-	a.RLock()
+	a.Lock()
 
-	if a.screen == nil {
+	screen := a.screen
+	if screen == nil {
 		// Screen has not yet been initialized.
-		a.RUnlock()
+		a.Unlock()
 		return false
 	}
 
-	// Enter suspended mode.
-	a.suspendMutex.Lock()
-	defer a.suspendMutex.Unlock()
-	a.RUnlock()
-	a.Stop()
-
-	// Deal with panics during suspended mode. Exit the program.
-	defer func() {
-		if p := recover(); p != nil {
-			fmt.Println(p)
-			os.Exit(1)
-		}
-	}()
+	// Enter suspended mode. Make a new screen here already so our event loop can
+	// continue.
+	a.screen = nil
+	a.running = false
+	screen.Fini()
+	a.Unlock()
 
 	// Wait for "f" to return.
 	f()
 
-	// Make a new screen and redraw.
+	// Initialize our new screen and draw the contents.
 	a.Lock()
 	var err error
 	a.screen, err = tcell.NewScreen()
@@ -281,7 +310,9 @@ func (a *Application) Suspend(f func()) bool {
 	}
 	a.running = true
 	a.Unlock()
-	a.Draw()
+	a.draw()
+	a.suspendToken <- struct{}{}
+	// One key event will get lost, see https://github.com/gdamore/tcell/issues/194
 
 	// Continue application loop.
 	return true
@@ -290,6 +321,13 @@ func (a *Application) Suspend(f func()) bool {
 // Draw refreshes the screen. It calls the Draw() function of the application's
 // root primitive and then syncs the screen buffer.
 func (a *Application) Draw() *Application {
+	// We actually just queue this draw.
+	a.redraw <- struct{}{}
+	return a
+}
+
+// draw actually does what Draw() promises to do.
+func (a *Application) draw() *Application {
 	a.Lock()
 	defer a.Unlock()
 
@@ -431,14 +469,23 @@ func (a *Application) GetFocus() Primitive {
 	return a.focus
 }
 
-// QueueUpdate is used to synchronize changes to primitives by carrying an update function from separate goroutine to the Application event loop via channel
+// QueueUpdate is used to synchronize access to primitives from non-main
+// goroutines. The provided function will be executed as part of the event loop
+// and thus will not cause race conditions with other such update functions or
+// the Draw() function.
+//
+// Note that Draw() is not implicitly called after the execution of f as that
+// may not be desirable. You can call Draw() from f if the screen should be
+// refreshed after each update.
 func (a *Application) QueueUpdate(f func()) *Application {
 	a.updates <- f
 	return a
 }
 
-// QueueEvent takes an Event instance and sends it to the Application event loop via channel
-func (a *Application) QueueEvent(e tcell.Event) *Application {
-	a.events <- e
+// QueueEvent sends an event to the Application event loop.
+//
+// It is not recommended for event to be nil.
+func (a *Application) QueueEvent(event tcell.Event) *Application {
+	a.events <- event
 	return a
 }
diff --git a/demos/presentation/textview.go b/demos/presentation/textview.go
index 65f2d2c..7d06035 100644
--- a/demos/presentation/textview.go
+++ b/demos/presentation/textview.go
@@ -34,10 +34,13 @@ func TextView1(nextSlide func()) (title string, content tview.Primitive) {
 	textView := tview.NewTextView().
 		SetTextColor(tcell.ColorYellow).
 		SetScrollable(false).
-		SetChangedFunc(func() {
+		SetDoneFunc(func(key tcell.Key) {
+			nextSlide()
+		})
+	textView.SetChangedFunc(func() {
+		if textView.HasFocus() {
 			app.Draw()
-		}).SetDoneFunc(func(key tcell.Key) {
-		nextSlide()
+		}
 	})
 	go func() {
 		var n int
diff --git a/doc.go b/doc.go
index 1b51a27..2e7d06c 100644
--- a/doc.go
+++ b/doc.go
@@ -137,6 +137,27 @@ Unicode Support
 
 This package supports unicode characters including wide characters.
 
+Concurrency
+
+Many functions in this package are not thread-safe. For many applications, this
+may not be an issue: If your code makes changes in response to key events, it
+will execute in the main goroutine and thus will not cause any race conditions.
+
+If you access your primitives from other goroutines, however, you will need to
+synchronize execution. The easiest way to do this is to call
+Application.QueueUpdate() (see its documentation for details):
+
+  go func() {
+    app.QueueUpdate(func() {
+      table.SetCellSimple(0, 0, "Foo bar")
+      app.Draw()
+    })
+  }()
+
+One exception to this is the io.Writer interface implemented by TextView. You
+can safely write to a TextView from any goroutine. See the TextView
+documentation for details.
+
 Type Hierarchy
 
 All widgets listed above contain the Box type. All of Box's functions are
diff --git a/textview.go b/textview.go
index d3c5f5e..7168990 100644
--- a/textview.go
+++ b/textview.go
@@ -31,7 +31,7 @@ type textViewIndex struct {
 // TextView is a box which displays text. It implements the io.Writer interface
 // so you can stream text to it. This does not trigger a redraw automatically
 // but if a handler is installed via SetChangedFunc(), you can cause it to be
-// redrawn.
+// redrawn. (See SetChangedFunc() for more details.)
 //
 // Navigation
 //
@@ -260,8 +260,20 @@ func (t *TextView) SetRegions(regions bool) *TextView {
 }
 
 // SetChangedFunc sets a handler function which is called when the text of the
-// text view has changed. This is typically used to cause the application to
-// redraw the screen.
+// text view has changed. This is useful when text is written to this io.Writer
+// in a separate goroutine. This does not automatically cause the screen to be
+// refreshed so you may want to use the "changed" handler to redraw the screen.
+//
+// Note that to avoid race conditions or deadlocks, there are a few rules you
+// should follow:
+//
+//   - You can call Application.Draw() from this handler.
+//   - You can call TextView.HasFocus() from this handler.
+//   - During the execution of this handler, access to any other variables from
+//     this primitive or any other primitive should be queued using
+//     Application.QueueUpdate().
+//
+// See package description for details on dealing with concurrency.
 func (t *TextView) SetChangedFunc(handler func()) *TextView {
 	t.changed = handler
 	return t
@@ -441,13 +453,33 @@ func (t *TextView) GetRegionText(regionID string) string {
 	return escapePattern.ReplaceAllString(buffer.String(), `[$1$2]`)
 }
 
+// Focus is called when this primitive receives focus.
+func (t *TextView) Focus(delegate func(p Primitive)) {
+	// Implemented here with locking because this is used by layout primitives.
+	t.Lock()
+	defer t.Unlock()
+	t.hasFocus = true
+}
+
+// HasFocus returns whether or not this primitive has focus.
+func (t *TextView) HasFocus() bool {
+	// Implemented here with locking because this may be used in the "changed"
+	// callback.
+	t.Lock()
+	defer t.Unlock()
+	return t.hasFocus
+}
+
 // Write lets us implement the io.Writer interface. Tab characters will be
 // replaced with TabSize space characters. A "\n" or "\r\n" will be interpreted
 // as a new line.
 func (t *TextView) Write(p []byte) (n int, err error) {
 	// Notify at the end.
-	if t.changed != nil {
-		defer t.changed()
+	t.Lock()
+	changed := t.changed
+	t.Unlock()
+	if changed != nil {
+		defer changed() // Deadlocks may occur if we lock here.
 	}
 
 	t.Lock()