refactor(tracker): new closing mechanism logic

cmd/sointu-track/main.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"runtime/pprof"
+	"time"
 
 	"gioui.org/app"
 	"github.com/vsariola/sointu"
@@ -71,7 +72,8 @@ func main() {
 	go func() {
 		trackerUi.Main()
 		audioCloser.Close()
-		detector.Close()
+		tracker.TrySend(broker.CloseDetector, struct{}{})
+		tracker.TimeoutReceive(broker.FinishedDetector, 3*time.Second)
 		if *cpuprofile != "" {
 			pprof.StopCPUProfile()
 			f.Close()

cmd/sointu-vsti/main.go

@@ -9,6 +9,7 @@ import (
 	"math"
 	"os"
 	"path/filepath"
+	"time"
 
 	"github.com/vsariola/sointu"
 	"github.com/vsariola/sointu/cmd"
@@ -134,17 +135,22 @@ func init() {
 					}
 				},
 				CloseFunc: func() {
-					broker.ToModel <- tracker.MsgToModel{Data: func() { t.ForceQuit().Do() }}
-					t.WaitQuitted()
-					detector.Close()
+					tracker.TrySend(broker.CloseDetector, struct{}{})
+					tracker.TrySend(broker.CloseGUI, struct{}{})
+					tracker.TimeoutReceive(broker.FinishedDetector, 3*time.Second)
+					tracker.TimeoutReceive(broker.FinishedGUI, 3*time.Second)
 				},
 				GetChunkFunc: func(isPreset bool) []byte {
 					retChn := make(chan []byte)
-					broker.ToModel <- tracker.MsgToModel{Data: func() { retChn <- t.MarshalRecovery() }}
-					return <-retChn
+
+					if !tracker.TrySend(broker.ToModel, tracker.MsgToModel{Data: func() { retChn <- t.MarshalRecovery() }}) {
+						return nil
+					}
+					ret, _ := tracker.TimeoutReceive(retChn, 5*time.Second) // ret will be nil if timeout or channel closed
+					return ret
 				},
 				SetChunkFunc: func(data []byte, isPreset bool) {
-					broker.ToModel <- tracker.MsgToModel{Data: func() { t.UnmarshalRecovery(data) }}
+					tracker.TrySend(broker.ToModel, tracker.MsgToModel{Data: func() { t.UnmarshalRecovery(data) }})
 				},
 			}
 

tracker/action.go

@@ -436,10 +436,12 @@ func (m *Model) OpenSong() Action {
 	})
 }
 
-func (m *Model) Quit() Action {
+func (m *Model) RequestQuit() Action {
 	return Allow(func() {
+		if !m.quitted {
 			m.dialog = QuitChanges
 			m.completeAction(true)
+		}
 	})
 }
 

tracker/broker.go

@@ -2,6 +2,7 @@ package tracker
 
 import (
 	"sync"
+	"time"
 
 	"github.com/vsariola/sointu"
 	"github.com/vsariola/sointu/vm"
@@ -16,11 +17,33 @@ type (
 	// return buffers to pass buffers around without allocating new memory every
 	// time. We can later consider making many-to-many types of communication
 	// and more complex routing logic to the Broker if needed.
+	//
+	// For closing goroutines, the broker has two channels for each goroutine:
+	// CloseXXX and FinishedXXX. The CloseXXX channel has a capacity of 1, so
+	// you can always send a empty message (struct{}{}) to it without blocking.
+	// If the channel is already full, that means someone else has already
+	// requested its closure and the goroutine is already closing, so dropping
+	// the message is fine. Then, FinishedXXX is used to signal that a goroutine
+	// has succesfully closed and cleaned up. Nothing is ever sent to the
+	// channel, it is only closed. You can wait until the goroutines is done
+	// closing with "<- FinishedXXX", which for avoiding deadlocks can be
+	// combined with a timeout:
+	//    select {
+	//      case <-FinishedXXX:
+	//      case <-time.After(3 * time.Second):
+	//    }
+
 	Broker struct {
 		ToModel    chan MsgToModel
 		ToPlayer   chan any // TODO: consider using a sum type here, for a bit more type safety. See: https://www.jerf.org/iri/post/2917/
 		ToDetector chan MsgToDetector
 
+		CloseDetector chan struct{}
+		CloseGUI      chan struct{}
+
+		FinishedGUI      chan struct{}
+		FinishedDetector chan struct{}
+
 		bufferPool sync.Pool
 	}
 
@@ -49,7 +72,6 @@ type (
 	// which gets executed in the detector goroutine.
 	MsgToDetector struct {
 		Reset bool
-		Quit  bool
 		Data  any // TODO: consider using a sum type here, for a bit more type safety. See: https://www.jerf.org/iri/post/2917/
 
 		WeightingType    WeightingType
@@ -64,6 +86,10 @@ func NewBroker() *Broker {
 		ToPlayer:         make(chan interface{}, 1024),
 		ToModel:          make(chan MsgToModel, 1024),
 		ToDetector:       make(chan MsgToDetector, 1024),
+		CloseDetector:    make(chan struct{}, 1),
+		CloseGUI:         make(chan struct{}, 1),
+		FinishedGUI:      make(chan struct{}),
+		FinishedDetector: make(chan struct{}),
 		bufferPool:       sync.Pool{New: func() interface{} { return &sointu.AudioBuffer{} }},
 	}
 }
@@ -96,3 +122,15 @@ func TrySend[T any](c chan<- T, v T) bool {
 	}
 	return true
 }
+
+// TimeoutReceive is a helper function to block until a value is received from a
+// channel, or timing out after t. ok will be false if the timeout occurred or
+// if the channel is closed.
+func TimeoutReceive[T any](c <-chan T, t time.Duration) (v T, ok bool) {
+	select {
+	case v, ok = <-c:
+		return v, ok
+	case <-time.After(t):
+		return v, false
+	}
+}

tracker/detector.go

@@ -12,6 +12,7 @@ type (
 		broker           *Broker
 		loudnessDetector loudnessDetector
 		peakDetector     peakDetector
+		chunkHistory     sointu.AudioBuffer
 	}
 
 	WeightingType int
@@ -98,15 +99,22 @@ func NewDetector(b *Broker) *Detector {
 }
 
 func (s *Detector) Run() {
-	var chunkHistory sointu.AudioBuffer
-	for msg := range s.broker.ToDetector {
+	for {
+		select {
+		case <-s.broker.CloseDetector:
+			close(s.broker.FinishedDetector)
+			return
+		case msg := <-s.broker.ToDetector:
+			s.handleMsg(msg)
+		}
+	}
+}
+
+func (s *Detector) handleMsg(msg MsgToDetector) {
 	if msg.Reset {
 		s.loudnessDetector.reset()
 		s.peakDetector.reset()
 	}
-		if msg.Quit {
-			return
-		}
 	if msg.HasWeightingType {
 		s.loudnessDetector.weighting = weightings[WeightingType(msg.WeightingType)]
 		s.loudnessDetector.reset()
@@ -121,21 +129,21 @@ func (s *Detector) Run() {
 		buf := *data
 		for {
 			var chunk sointu.AudioBuffer
-				if len(chunkHistory) > 0 && len(chunkHistory) < 4410 {
-					l := min(len(buf), 4410-len(chunkHistory))
-					chunkHistory = append(chunkHistory, buf[:l]...)
-					if len(chunkHistory) < 4410 {
+			if len(s.chunkHistory) > 0 && len(s.chunkHistory) < 4410 {
+				l := min(len(buf), 4410-len(s.chunkHistory))
+				s.chunkHistory = append(s.chunkHistory, buf[:l]...)
+				if len(s.chunkHistory) < 4410 {
 					break
 				}
-					chunk = chunkHistory
+				chunk = s.chunkHistory
 				buf = buf[l:]
 			} else {
 				if len(buf) >= 4410 {
 					chunk = buf[:4410]
 					buf = buf[4410:]
 				} else {
-						chunkHistory = chunkHistory[:0]
-						chunkHistory = append(chunkHistory, buf...)
+					s.chunkHistory = s.chunkHistory[:0]
+					s.chunkHistory = append(s.chunkHistory, buf...)
 					break
 				}
 			}
@@ -147,17 +155,8 @@ func (s *Detector) Run() {
 				},
 			})
 		}
-			s.broker.PutAudioBuffer(data)
-		case func():
-			data()
 	}
 }
-}
-
-// Close may theoretically block if the broker is full, but it should not happen in practice
-func (s *Detector) Close() {
-	s.broker.ToDetector <- MsgToDetector{Quit: true}
-}
 
 func makeLoudnessDetector(weighting WeightingType) loudnessDetector {
 	return loudnessDetector{

tracker/gioui/keyevent.go

@@ -184,7 +184,7 @@ func (t *Tracker) KeyEvent(e key.Event, gtx C) {
 		t.OpenSong().Do()
 	case "Quit":
 		if canQuit {
-			t.Quit().Do()
+			t.RequestQuit().Do()
 		}
 	case "SaveSong":
 		t.SaveSong().Do()

tracker/gioui/songpanel.go

@@ -336,7 +336,7 @@ func NewMenuBar(model *tracker.Model) *MenuBar {
 		{IconBytes: icons.ImageAudiotrack, Text: "Export Wav...", ShortcutText: keyActionMap["ExportWav"], Doer: model.Export()},
 	}
 	if canQuit {
-		ret.fileMenuItems = append(ret.fileMenuItems, MenuItem{IconBytes: icons.ActionExitToApp, Text: "Quit", ShortcutText: keyActionMap["Quit"], Doer: model.Quit()})
+		ret.fileMenuItems = append(ret.fileMenuItems, MenuItem{IconBytes: icons.ActionExitToApp, Text: "Quit", ShortcutText: keyActionMap["Quit"], Doer: model.RequestQuit()})
 	}
 	ret.editMenuItems = []MenuItem{
 		{IconBytes: icons.ContentUndo, Text: "Undo", ShortcutText: keyActionMap["Undo"], Doer: model.Undo()},

tracker/gioui/tracker.go

@@ -5,7 +5,6 @@ import (
 	"image"
 	"io"
 	"path/filepath"
-	"sync"
 	"time"
 
 	"gioui.org/app"
@@ -50,7 +49,6 @@ type (
 
 		filePathString tracker.String
 
-		quitWG      sync.WaitGroup
 		execChan    chan func()
 		preferences Preferences
 
@@ -102,85 +100,21 @@ func NewTracker(model *tracker.Model) *Tracker {
 	t.Theme.Palette.Fg = primaryColor
 	t.Theme.Palette.ContrastFg = black
 	t.TrackEditor.scrollTable.Focus()
-	t.quitWG.Add(1)
 	return t
 }
 
 func (t *Tracker) Main() {
-	titleFooter := ""
-	w := t.newWindow()
 	t.InstrumentEditor.Focus()
 	recoveryTicker := time.NewTicker(time.Second * 30)
-	t.Explorer = explorer.NewExplorer(w)
-	// Make a channel to read window events from.
-	events := make(chan event.Event)
-	// Make a channel to signal the end of processing a window event.
-	acks := make(chan struct{})
-	go eventLoop(w, events, acks)
 	var ops op.Ops
-	for {
-		select {
-		case e := <-t.Broker().ToModel:
-			t.ProcessMsg(e)
-			w.Invalidate()
-		case e := <-events:
-			switch e := e.(type) {
-			case app.DestroyEvent:
-				acks <- struct{}{}
-				if canQuit {
-					t.Quit().Do()
-				}
-				if !t.Quitted() {
-					// TODO: uh oh, there's no way of canceling the destroyevent in gioui? so we create a new window just to show the dialog
-					w = t.newWindow()
+	titlePath := ""
+	for !t.Quitted() {
+		w := t.newWindow()
+		w.Option(app.Title(titleFromPath(titlePath)))
 		t.Explorer = explorer.NewExplorer(w)
-					go eventLoop(w, events, acks)
-				}
-			case app.FrameEvent:
-				if titleFooter != t.filePathString.Value() {
-					titleFooter = t.filePathString.Value()
-					if titleFooter != "" {
-						w.Option(app.Title(fmt.Sprintf("Sointu Tracker - %v", titleFooter)))
-					} else {
-						w.Option(app.Title("Sointu Tracker"))
-					}
-				}
-				gtx := app.NewContext(&ops, e)
-				if t.Playing().Value() && t.Follow().Value() {
-					t.TrackEditor.scrollTable.RowTitleList.CenterOn(t.PlaySongRow())
-				}
-				t.Layout(gtx, w)
-				e.Frame(gtx.Ops)
-				acks <- struct{}{}
-			default:
-				acks <- struct{}{}
-			}
-		case <-recoveryTicker.C:
-			t.SaveRecovery()
-		}
-		if t.Quitted() {
-			break
-		}
-	}
-	recoveryTicker.Stop()
-	w.Perform(system.ActionClose)
-	t.SaveRecovery()
-	t.quitWG.Done()
-}
-
-func (t *Tracker) newWindow() *app.Window {
-	w := new(app.Window)
-	w.Option(app.Title("Sointu Tracker"))
-	w.Option(app.Size(t.preferences.WindowSize()))
-	if t.preferences.Window.Maximized {
-		w.Option(app.Maximized.Option())
-	}
-	return w
-}
-
-func eventLoop(w *app.Window, events chan<- event.Event, acks <-chan struct{}) {
-	// Iterate window events, sending each to the old event loop and waiting for
-	// a signal that processing is complete before iterating again.
+		acks := make(chan struct{})
+		events := make(chan event.Event)
+		go func() {
 			for {
 				ev := w.Event()
 				events <- ev
@@ -189,10 +123,64 @@ func eventLoop(w *app.Window, events chan<- event.Event, acks <-chan struct{}) {
 					return
 				}
 			}
+		}()
+	F:
+		for {
+			select {
+			case e := <-t.Broker().ToModel:
+				t.ProcessMsg(e)
+				w.Invalidate()
+			case <-t.Broker().CloseGUI:
+				t.ForceQuit().Do()
+				w.Perform(system.ActionClose)
+			case e := <-events:
+				switch e := e.(type) {
+				case app.DestroyEvent:
+					if canQuit {
+						t.RequestQuit().Do()
+					}
+					acks <- struct{}{}
+					break F // this window is done, we need to create a new one
+				case app.FrameEvent:
+					if titlePath != t.filePathString.Value() {
+						titlePath = t.filePathString.Value()
+						w.Option(app.Title(titleFromPath(titlePath)))
+					}
+					gtx := app.NewContext(&ops, e)
+					if t.Playing().Value() && t.Follow().Value() {
+						t.TrackEditor.scrollTable.RowTitleList.CenterOn(t.PlaySongRow())
+					}
+					t.Layout(gtx, w)
+					e.Frame(gtx.Ops)
+					if t.Quitted() {
+						w.Perform(system.ActionClose)
+					}
+				}
+				acks <- struct{}{}
+			case <-recoveryTicker.C:
+				t.SaveRecovery()
+			}
+		}
+	}
+	recoveryTicker.Stop()
+	t.SaveRecovery()
+	close(t.Broker().FinishedGUI)
 }
 
-func (t *Tracker) WaitQuitted() {
-	t.quitWG.Wait()
+func (t *Tracker) newWindow() *app.Window {
+	w := new(app.Window)
+	w.Option(app.Size(t.preferences.WindowSize()))
+	if t.preferences.Window.Maximized {
+		w.Option(app.Maximized.Option())
+	}
+	return w
+}
+
+func titleFromPath(path string) string {
+	if path == "" {
+		return "Sointu Tracker"
+	}
+	return fmt.Sprintf("Sointu Tracker - %s", path)
 }
 
 func (t *Tracker) Layout(gtx layout.Context, w *app.Window) {

tracker/model_test.go

@@ -309,6 +309,6 @@ func FuzzModel(f *testing.F) {
 			}
 		}
 		closeChan <- struct{}{}
-		broker.ToDetector <- tracker.MsgToDetector{Quit: true}
+		broker.CloseDetector <- struct{}{}
 	})
 }