~sircmpwn/aerc

Add closeMutex to avoid crash v1 PROPOSED

y0ast: 1
 Add closeMutex to avoid crash

 1 files changed, 7 insertions(+), 2 deletions(-)
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/aerc/patches/16362/mbox | git am -3
Learn more about email & git
View this thread in the archives

[PATCH] Add closeMutex to avoid crash Export this patch

Currently the terminal widget can be closed while it's also being
updated leading to a SIGSEGV: https://todo.sr.ht/~sircmpwn/aerc2/449.

This patch adds the minimal amount of locking to make that impossible.
---
 widgets/terminal.go | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/widgets/terminal.go b/widgets/terminal.go
index 0277521..8618b1e 100644
--- a/widgets/terminal.go
+++ b/widgets/terminal.go
@@ -9,7 +9,7 @@ import (
	"git.sr.ht/~sircmpwn/aerc/lib/ui"

	"github.com/creack/pty"
	"github.com/ddevault/go-libvterm"
	vterm "github.com/ddevault/go-libvterm"
	"github.com/gdamore/tcell/v2"
)

@@ -105,6 +105,7 @@ type Terminal struct {
	damage      []vterm.Rect // protected by damageMutex
	damageMutex sync.Mutex
	writeMutex  sync.Mutex
	closeMutex  sync.Mutex // Only necessary around cmd/pty changes

	OnClose func(err error)
	OnEvent func(event tcell.Event) bool
@@ -171,6 +172,7 @@ func (term *Terminal) flushTerminal() {
}

func (term *Terminal) Close(err error) {
	term.closeMutex.Lock()
	if term.closed {
		return
	}
@@ -184,10 +186,11 @@ func (term *Terminal) Close(err error) {
		term.cmd.Wait()
		term.cmd = nil
	}
	if !term.closed && term.OnClose != nil {
	if term.OnClose != nil {
		term.OnClose(err)
	}
	term.closed = true
	term.closeMutex.Unlock()
	term.ctx.HideCursor()
}

@@ -227,6 +230,7 @@ func (term *Terminal) Draw(ctx *ui.Context) {

	term.ctx = ctx // gross

	term.closeMutex.Lock()
	if !term.closed {
		winsize := pty.Winsize{
			Cols: uint16(ctx.Width()),
@@ -266,6 +270,7 @@ func (term *Terminal) Draw(ctx *ui.Context) {
			return
		}
	}
	term.closeMutex.Unlock()

	screen := term.vterm.ObtainScreen()

-- 
2.24.3 (Apple Git-128)
Upon further testing, this patch sadly leads to a deadlock, will investigate and put up a v2.
Hi Jozzle,

Are you sure this isn't deadlocking?
Because it should, based on how you wrote it ;)