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 <tim@timculverhouse.com> Acked-by: Robin Jarry <robin@jarry.cc>
This commit is contained in:
parent
176a3d989f
commit
020279108c
3 changed files with 14 additions and 38 deletions
2
go.mod
2
go.mod
|
@ -3,7 +3,7 @@ module git.sr.ht/~rjarry/aerc
|
||||||
go 1.16
|
go 1.16
|
||||||
|
|
||||||
require (
|
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
|
git.sr.ht/~sircmpwn/getopt v1.0.0
|
||||||
github.com/ProtonMail/go-crypto v0.0.0-20211221144345-a4f6767435ab
|
github.com/ProtonMail/go-crypto v0.0.0-20211221144345-a4f6767435ab
|
||||||
github.com/arran4/golang-ical v0.0.0-20220517104411-fd89fefb0182
|
github.com/arran4/golang-ical v0.0.0-20220517104411-fd89fefb0182
|
||||||
|
|
4
go.sum
4
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=
|
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=
|
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=
|
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.2.0 h1:aiRmFm6FLqdZOKVUYNumh0K1n7meaebU4BvzVGa1LNA=
|
||||||
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/go.mod h1:0CEvSJrV0ItwHlPHSmkxd5egiuUtpvGnnCsqzV02BN4=
|
||||||
git.sr.ht/~sircmpwn/getopt v1.0.0 h1:/pRHjO6/OCbBF4puqD98n6xtPEgE//oq5U8NXjP7ROc=
|
git.sr.ht/~sircmpwn/getopt v1.0.0 h1:/pRHjO6/OCbBF4puqD98n6xtPEgE//oq5U8NXjP7ROc=
|
||||||
git.sr.ht/~sircmpwn/getopt v1.0.0/go.mod h1:wMEGFFFNuPos7vHmWXfszqImLppbc0wEhh6JBfJIUgw=
|
git.sr.ht/~sircmpwn/getopt v1.0.0/go.mod h1:wMEGFFFNuPos7vHmWXfszqImLppbc0wEhh6JBfJIUgw=
|
||||||
github.com/Antonboom/errname v0.1.7 h1:mBBDKvEYwPl4WFFNwec1CZO096G6vzK9vvDQzAwkako=
|
github.com/Antonboom/errname v0.1.7 h1:mBBDKvEYwPl4WFFNwec1CZO096G6vzK9vvDQzAwkako=
|
||||||
|
|
|
@ -42,24 +42,9 @@ func (term *Terminal) Close(err error) {
|
||||||
if term.closed {
|
if term.closed {
|
||||||
return
|
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 {
|
if term.vterm != nil {
|
||||||
|
// Stop receiving events
|
||||||
|
term.vterm.Unwatch(term)
|
||||||
term.vterm.Close()
|
term.vterm.Close()
|
||||||
}
|
}
|
||||||
if !term.closed && term.OnClose != nil {
|
if !term.closed && term.OnClose != nil {
|
||||||
|
@ -96,25 +81,14 @@ func (term *Terminal) Draw(ctx *ui.Context) {
|
||||||
term.ctx = ctx // gross
|
term.ctx = ctx // gross
|
||||||
term.vterm.SetView(ctx.View())
|
term.vterm.SetView(ctx.View())
|
||||||
if !term.running && !term.closed && term.cmd != nil {
|
if !term.running && !term.closed && term.cmd != nil {
|
||||||
go func() {
|
term.vterm.Watch(term)
|
||||||
defer logging.PanicHandler()
|
attr := &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 1}
|
||||||
term.vterm.Watch(term)
|
if err := term.vterm.StartWithAttrs(term.cmd, attr); err != nil {
|
||||||
attr := &syscall.SysProcAttr{Setsid: true, Setctty: true, Ctty: 1}
|
logging.Errorf("error running terminal: %w", err)
|
||||||
if err := term.vterm.RunWithAttrs(term.cmd, attr); err != nil {
|
term.Close(err)
|
||||||
logging.Errorf("error running terminal: %w", err)
|
return
|
||||||
term.Close(err)
|
|
||||||
term.running = false
|
|
||||||
return
|
|
||||||
}
|
|
||||||
term.running = false
|
|
||||||
term.Close(nil)
|
|
||||||
}()
|
|
||||||
for {
|
|
||||||
if term.cmd.Process != nil {
|
|
||||||
term.running = true
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
term.running = true
|
||||||
if term.OnStart != nil {
|
if term.OnStart != nil {
|
||||||
term.OnStart()
|
term.OnStart()
|
||||||
}
|
}
|
||||||
|
@ -193,6 +167,8 @@ func (term *Terminal) HandleEvent(ev tcell.Event) bool {
|
||||||
if term.OnTitle != nil {
|
if term.OnTitle != nil {
|
||||||
term.OnTitle(ev.Title())
|
term.OnTitle(ev.Title())
|
||||||
}
|
}
|
||||||
|
case *tcellterm.EventClosed:
|
||||||
|
term.Close(nil)
|
||||||
}
|
}
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue