Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventError fired after suspended Screen resumes #480

Closed
rivo opened this issue Aug 22, 2021 · 11 comments
Closed

EventError fired after suspended Screen resumes #480

rivo opened this issue Aug 22, 2021 · 11 comments
Labels

Comments

@rivo
Copy link
Contributor

rivo commented Aug 22, 2021

In the past, in tview, I hadn't handled the EventError event. But as one user reported, this gets fired when the user closes the terminal when the application is still running so it should be handled.

Now it turns out I'm getting EventError after a suspended screen resumes. Here's some code reproduce this:

package main

import (
	"fmt"
	"os"
	"os/exec"

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

func main() {
	s, e := tcell.NewScreen()
	if e != nil {
		fmt.Fprintf(os.Stderr, "%v\n", e)
		os.Exit(1)
	}
	if e := s.Init(); e != nil {
		fmt.Fprintf(os.Stderr, "%v\n", e)
		os.Exit(1)
	}

	for {
		switch ev := s.PollEvent().(type) {
		case *tcell.EventError:
			s.Fini()
			panic(ev) // This fires with EOF after we return from vim.
		case *tcell.EventKey:
			if ev.Key() == tcell.KeyEscape {
				s.Fini()
				os.Exit(0)
			} else if ev.Rune() == 's' { // Press "s" to suspend and start vim.
				if err := s.Suspend(); err != nil {
					panic(err)
				}

				cmd := exec.Command("vim", "/tmp/foo")
				cmd.Stdout = os.Stdout
				cmd.Stdin = os.Stdin
				_ = cmd.Run()

				if err := s.Resume(); err != nil {
					panic(err)
				}
			}
		}
	}
}

Press s to suspend the screen and start vim. When you exit vim, the application panics with "EOF" after the EventError is handled. (It's not vim, this happens with any command.)

Any idea why this happens?

@dundee
Copy link

dundee commented Sep 3, 2021

It panics on panic: read /dev/tty: i/o timeout on ArchLinux.

require (
	github.com/gdamore/tcell/v2 v2.4.0
	github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
	github.com/mattn/go-runewidth v0.0.13 // indirect
	golang.org/x/sys v0.0.0-20210903071746-97244b99971b // indirect
	golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b // indirect
	golang.org/x/text v0.3.7 // indirect
)
go version go1.17 linux/amd64

@gdamore
Copy link
Owner

gdamore commented Sep 3, 2021

What version of ArchLinux?

@gdamore
Copy link
Owner

gdamore commented Sep 3, 2021

Wait, did the terminal close? (E.g. you did this from a subshell?) I could see that being a problem. I'm not sure how best to fix this -- because we have to have a way to report this problem to the calling application. But clearly panic() isn't ideal.

@gdamore
Copy link
Owner

gdamore commented Sep 3, 2021

Ok , this isn't specific to Arch. I've reproduced in WSL.

@gdamore
Copy link
Owner

gdamore commented Sep 4, 2021

So the problem is when we suspend, we "close" the channel and rely on a time out to wake us up, but we also send a bogus error upstream. We can conditionalize sending the error upstream on t.running.

@gdamore
Copy link
Owner

gdamore commented Sep 4, 2021

Please have a try with the branch in fix480 -- I suspect this will address the problem.

@rivo
Copy link
Contributor Author

rivo commented Sep 4, 2021

It works on macOS, thank you.

@gdamore gdamore closed this as completed in f057f0a Sep 5, 2021
@dundee
Copy link

dundee commented Sep 5, 2021

It works on Arch now too. Thanks!

@gdamore
Copy link
Owner

gdamore commented Sep 27, 2021

This change actually created a worse regression -- as some activity to wake and exit the poller is required, and this prevents that from happening. This causes exiting mouse.go to hang until another key is pressed, and suspending also is not immediate. :(

@gdamore
Copy link
Owner

gdamore commented Sep 27, 2021

I'd be grateful if some macOS user could test the fix for the regression -- see PR #490

@rivo
Copy link
Contributor Author

rivo commented Sep 27, 2021

I just tried the code above on macOS with 0f66ee8 and it worked as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants