app.Suspend() deadlocks cview #58

Closed
opened 2 years ago by flowchartsman · 6 comments

Similar to this tview issue, calling app.Suspend() to shell out to a program like vim leaves an application in a completely unresponsive busy loop of some kind. The behavior is exactly the same, and can be easily reproduced with the following program, which I am running on MacOS Catalina:

// Demo code for the Form primitive.
package main

import (
	"os"
	"os/exec"

	"code.rocketnine.space/tslocum/cview"
)

func main() {
	app := cview.NewApplication()
	app.EnableMouse(true)

	form := cview.NewForm()
	form.AddDropDownSimple("Title", 0, nil, "Mr.", "Ms.", "Mrs.", "Dr.", "Prof.")
	form.AddInputField("First name", "", 20, nil, nil)
	form.AddInputField("Last name", "", 20, nil, nil)
	addressField := cview.NewInputField()
	addressField.SetLabel("Address")
	addressField.SetFieldWidth(30)
	addressField.SetFieldNote("Your complete address")
	form.AddFormItem(addressField)
	form.AddPasswordField("Password", "", 10, '*', nil)
	form.AddCheckBox("", "Age 18+", false, nil)
	form.AddButton("Launch vim", func() {
		app.Suspend(func() {
			cmd := exec.Command("vim", "~/tmp/test.txt")
			cmd.Stdout = os.Stdout
			cmd.Stdin = os.Stdin
			cmd.Stderr = os.Stderr
			cmd.Run()
		})
	})
	form.AddButton("Quit", func() {
		app.Stop()
	})
	form.SetBorder(true)
	form.SetTitle("Enter some data")
	form.SetTitleAlign(cview.AlignLeft)

	app.SetRoot(form, true)
	if err := app.Run(); err != nil {
		panic(err)
	}
}
Similar to [this tview issue](https://github.com/rivo/tview/issues/244), calling `app.Suspend()` to shell out to a program like vim leaves an application in a completely unresponsive busy loop of some kind. The behavior is exactly the same, and can be easily reproduced with the following program, which I am running on MacOS Catalina: ``` // Demo code for the Form primitive. package main import ( "os" "os/exec" "code.rocketnine.space/tslocum/cview" ) func main() { app := cview.NewApplication() app.EnableMouse(true) form := cview.NewForm() form.AddDropDownSimple("Title", 0, nil, "Mr.", "Ms.", "Mrs.", "Dr.", "Prof.") form.AddInputField("First name", "", 20, nil, nil) form.AddInputField("Last name", "", 20, nil, nil) addressField := cview.NewInputField() addressField.SetLabel("Address") addressField.SetFieldWidth(30) addressField.SetFieldNote("Your complete address") form.AddFormItem(addressField) form.AddPasswordField("Password", "", 10, '*', nil) form.AddCheckBox("", "Age 18+", false, nil) form.AddButton("Launch vim", func() { app.Suspend(func() { cmd := exec.Command("vim", "~/tmp/test.txt") cmd.Stdout = os.Stdout cmd.Stdin = os.Stdin cmd.Stderr = os.Stderr cmd.Run() }) }) form.AddButton("Quit", func() { app.Stop() }) form.SetBorder(true) form.SetTitle("Enter some data") form.SetTitleAlign(cview.AlignLeft) app.SetRoot(form, true) if err := app.Run(); err != nil { panic(err) } } ```
tslocum added the
bug
label 2 years ago
Poster

I'll note that debugging this issue is very tetchy, and I was only able to make the barest of headway by attaching to the running process with dlv, but, funnily enough, stepping through did not reproduce the issue, and attempting to issue a debugger pause after it happend, found me at random locations within tcell or cview lower-level drawing functions, leading me to believe that this is a subtle race condition bug, and with the number of mutexes getting thrown around, I found it pretty daunting to continue, not being familiar with either codebase.

I'll note that debugging this issue is very tetchy, and I was only able to make the barest of headway by attaching to the running process with `dlv`, but, funnily enough, stepping through did not reproduce the issue, and attempting to issue a debugger pause after it happend, found me at random locations within tcell or cview lower-level drawing functions, leading me to believe that this is a subtle race condition bug, and with the number of mutexes getting thrown around, I found it pretty daunting to continue, not being familiar with either codebase.
Owner

Thanks for reporting this. I have updated Application.Suspend to suspend and resume the screen, rather than finalize it and create an entirely new screen. This gets us closer to what we want, but there is still an issue. When suspending, the screen waits for a mouse or key event before actually suspending. I have an idea of why this is (blocking on PollEvent causes the suspend to hang until PollEvent returns), but I will need to spend some more time on this to be sure.

Thanks for reporting this. I have updated Application.Suspend to suspend and resume the screen, rather than finalize it and create an entirely new screen. This gets us closer to what we want, but there is still an issue. When suspending, the screen waits for a mouse or key event before actually suspending. I have an idea of why this is (blocking on PollEvent causes the suspend to hang until PollEvent returns), but I will need to spend some more time on this to be sure.
Poster

So does this mean it will appear to the user to freeze until a key is hit? Hmm. Is there an empty event that can be sent to PollEvent that would obviate that?

So does this mean it will appear to the user to freeze until a key is hit? Hmm. Is there an empty event that can be sent to PollEvent that would obviate that?
Owner

I've confirmed that the inputLoop is the issue. When suspending, the mainLoop and inputLoop must both return. inputLoop will only return once it has read something, because it blocks on Read. Rather than polling stdin directly, /dev/tty is polled (on UNIX at least). I have tried opening /dev/tty in non-blocking mode to no effect. I will bring this up with tcell's author and see if we can solve it together.

I've confirmed that the [inputLoop](https://github.com/gdamore/tcell/blob/299cb413dc87b63a30167f8a56f018e7fc67f391/tscreen.go#L1486) is the issue. When suspending, the `mainLoop` and `inputLoop` must both return. `inputLoop` will only return once it has read something, because it blocks on `Read`. Rather than polling stdin directly, `/dev/tty` is polled (on UNIX at least). I have tried opening `/dev/tty` in non-blocking mode to no effect. I will bring this up with tcell's author and see if we can solve it together.
Owner

With the recent commit, I've updated cview to only poll the screen between queued updates. This does not resolve the issue for some reason, as far as I can tell cview is copying the tcell mouse.go example. I will look into this more before submitting a bug in case I am missing something.

With the recent commit, I've updated cview to only poll the screen between queued updates. This does not resolve the issue for some reason, as far as I can tell cview is copying the tcell mouse.go example. I will look into this more before submitting a bug in case I am missing something.
Owner

I've submitted the following PR to resolve this upstream.

https://github.com/gdamore/tcell/pull/454

I've submitted the following PR to resolve this upstream. https://github.com/gdamore/tcell/pull/454
tslocum referenced this issue from a commit 2 years ago
tslocum closed this issue 2 years ago
tslocum referenced this issue from a commit 2 years ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: tslocum/cview#58
Loading…
There is no content yet.