From 020279108c4234974a26580183ef60945c5bd5cb Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Thu, 29 Sep 2022 17:14:59 -0500 Subject: [PATCH] terminal: fix race conditions on access to cmd Upgrade tcell-term to v0.2.0 Use Start method from tcell-term. This prevents aerc from needing to wait until the command has started to continue. The tcell-term start method blocks until the command is started, similar to cmd.Start. By doing so, we prevent a race condition between aerc and tcell-term on access to cmd.Process. Remove cleanup of cmd, this is all already handled by tcell-term when Close is called. Signed-off-by: Tim Culverhouse Acked-by: Robin Jarry --- go.mod | 2 +- go.sum | 4 ++-- widgets/terminal.go | 46 +++++++++++---------------------------------- 3 files changed, 14 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index 90aa1ce..fb05b46 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module git.sr.ht/~rjarry/aerc go 1.16 require ( - git.sr.ht/~rockorager/tcell-term v0.1.1-0.20220920200206-237e095b9b99 + git.sr.ht/~rockorager/tcell-term v0.2.0 git.sr.ht/~sircmpwn/getopt v1.0.0 github.com/ProtonMail/go-crypto v0.0.0-20211221144345-a4f6767435ab github.com/arran4/golang-ical v0.0.0-20220517104411-fd89fefb0182 diff --git a/go.sum b/go.sum index 6936556..3099f9a 100644 --- a/go.sum +++ b/go.sum @@ -62,8 +62,8 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 cloud.google.com/go/storage v1.14.0/go.mod h1:GrKmX003DSIwi9o29oFT7YDnHYwZoctc3fOKtUw0Xmo= contrib.go.opencensus.io/exporter/stackdriver v0.13.4/go.mod h1:aXENhDJ1Y4lIg4EUaVTwzvYETVNZk10Pu26tevFKLUc= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -git.sr.ht/~rockorager/tcell-term v0.1.1-0.20220920200206-237e095b9b99 h1:THSvF4k9Svq/dp9xZ8MgZ6d0FW5+GrXNUOj+YqfeqIY= -git.sr.ht/~rockorager/tcell-term v0.1.1-0.20220920200206-237e095b9b99/go.mod h1:0CEvSJrV0ItwHlPHSmkxd5egiuUtpvGnnCsqzV02BN4= +git.sr.ht/~rockorager/tcell-term v0.2.0 h1:aiRmFm6FLqdZOKVUYNumh0K1n7meaebU4BvzVGa1LNA= +git.sr.ht/~rockorager/tcell-term v0.2.0/go.mod h1:0CEvSJrV0ItwHlPHSmkxd5egiuUtpvGnnCsqzV02BN4= git.sr.ht/~sircmpwn/getopt v1.0.0 h1:/pRHjO6/OCbBF4puqD98n6xtPEgE//oq5U8NXjP7ROc= git.sr.ht/~sircmpwn/getopt v1.0.0/go.mod h1:wMEGFFFNuPos7vHmWXfszqImLppbc0wEhh6JBfJIUgw= github.com/Antonboom/errname v0.1.7 h1:mBBDKvEYwPl4WFFNwec1CZO096G6vzK9vvDQzAwkako= diff --git a/widgets/terminal.go b/widgets/terminal.go index cd90ed7..894bdf0 100644 --- a/widgets/terminal.go +++ b/widgets/terminal.go @@ -42,24 +42,9 @@ func (term *Terminal) Close(err error) { if term.closed { return } - // Stop receiving events - term.vterm.Unwatch(term) - if term.cmd != nil && term.cmd.Process != nil { - err := term.cmd.Process.Kill() - if err != nil { - logging.Warnf("failed to kill process: %v", err) - } - // Race condition here, check if cmd exists. If process exits - // fast, this could by nil and panic - if term.cmd != nil { - err = term.cmd.Wait() - } - if err != nil { - logging.Warnf("failed for wait for process to terminate: %v", err) - } - term.cmd = nil - } if term.vterm != nil { + // Stop receiving events + term.vterm.Unwatch(term) term.vterm.Close() } if !term.closed && term.OnClose != nil { @@ -96,25 +81,14 @@ func (term *Terminal) Draw(ctx *ui.Context) { term.ctx = ctx // gross term.vterm.SetView(ctx.View()) if !term.running && !term.closed && term.cmd != nil { - go func() { - defer logging.PanicHandler() - term.vterm.Watch(term) - attr := &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 1} - if err := term.vterm.RunWithAttrs(term.cmd, attr); err != nil { - logging.Errorf("error running terminal: %w", err) - term.Close(err) - term.running = false - return - } - term.running = false - term.Close(nil) - }() - for { - if term.cmd.Process != nil { - term.running = true - break - } + term.vterm.Watch(term) + attr := &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 1} + if err := term.vterm.StartWithAttrs(term.cmd, attr); err != nil { + logging.Errorf("error running terminal: %w", err) + term.Close(err) + return } + term.running = true if term.OnStart != nil { term.OnStart() } @@ -193,6 +167,8 @@ func (term *Terminal) HandleEvent(ev tcell.Event) bool { if term.OnTitle != nil { term.OnTitle(ev.Title()) } + case *tcellterm.EventClosed: + term.Close(nil) } return false }