Improve code quality and thread safety (#844)

- Format Go files (gofmt) for backend/windows/notify*.go
- Add thread safety to Stopwatch with sync.Mutex
- Add comprehensive unit tests for Stopwatch with race detection
- Add godoc comments to ImageCache public APIs
- Improve IPC documentation with platform-specific socket path details
- Document portable mode TODO for future enhancement

backend/imagecache.go

@@ -18,7 +18,9 @@ type CacheItem struct {
 	lastAccessed int64
 }
 
-// A custom in-memory cache for images with the following eviction strategy:
+// ImageCache is a thread-safe in-memory cache for images with LRU eviction.
+//
+// Eviction strategy:
 //  1. If there are fewer than MinSize items in the cache, none will be evicted
 //  2. If a new addition would make the cache exceed MaxSize, an item will be immediately evicted
 //     2a. in this case, evict the LRU expired item or if none expired, the LRU item
@@ -39,14 +41,19 @@ type ImageCache struct {
 	cache map[string]CacheItem
 }
 
+// ErrNotFound is returned when a requested cache item does not exist.
 var ErrNotFound = errors.New("item not found")
 
+// Init initializes the cache and starts a background goroutine for periodic eviction.
+// The goroutine stops when the provided context is cancelled.
 func (i *ImageCache) Init(ctx context.Context, evictionInterval time.Duration) {
 	i.cache = make(map[string]CacheItem)
 	go i.periodicallyEvict(ctx, evictionInterval)
 }
 
-// holds writer lock for O(i.MaxSize) worst case
+// SetWithTTL stores an image in the cache with a custom time-to-live duration.
+// If the cache is at MaxSize, an item will be evicted using LRU strategy.
+// Thread-safe. Holds writer lock for O(MaxSize) worst case.
 func (i *ImageCache) SetWithTTL(key string, val image.Image, ttl time.Duration) {
 	i.mu.Lock()
 	defer i.mu.Unlock()
@@ -71,10 +78,14 @@ func (i *ImageCache) SetWithTTL(key string, val image.Image, ttl time.Duration)
 	}
 }
 
+// Set stores an image in the cache with the default TTL.
+// See SetWithTTL for more details.
 func (i *ImageCache) Set(key string, val image.Image) {
 	i.SetWithTTL(key, val, i.DefaultTTL)
 }
 
+// Has returns true if the key exists in the cache, expired or not.
+// Thread-safe.
 func (i *ImageCache) Has(key string) bool {
 	i.mu.RLock()
 	defer i.mu.RUnlock()
@@ -83,10 +94,17 @@ func (i *ImageCache) Has(key string) bool {
 	return ok
 }
 
+// Get retrieves an image from the cache and updates its last accessed time.
+// Returns ErrNotFound if the key doesn't exist.
+// Thread-safe.
 func (i *ImageCache) Get(key string) (image.Image, error) {
 	return i.GetResetTTL(key, false)
 }
 
+// GetResetTTL retrieves an image and optionally resets its expiration time.
+// If resetTTL is true, the expiration is reset to now + original TTL.
+// Returns ErrNotFound if the key doesn't exist.
+// Thread-safe.
 func (i *ImageCache) GetResetTTL(key string, resetTTL bool) (image.Image, error) {
 	i.mu.Lock()
 	defer i.mu.Unlock()
@@ -102,7 +120,11 @@ func (i *ImageCache) GetResetTTL(key string, resetTTL bool) (image.Image, error)
 	return nil, ErrNotFound
 }
 
-// Gets the image if it exists and extends TTL to time.Now + ttl iff the image would expire before then
+// GetExtendTTL retrieves an image and extends its TTL if it would expire sooner.
+// The expiration time is extended to now + ttl only if the current expiration
+// is earlier than that time.
+// Returns ErrNotFound if the key doesn't exist.
+// Thread-safe.
 func (i *ImageCache) GetExtendTTL(key string, ttl time.Duration) (image.Image, error) {
 	i.mu.Lock()
 	defer i.mu.Unlock()
@@ -118,6 +140,10 @@ func (i *ImageCache) GetExtendTTL(key string, ttl time.Duration) (image.Image, e
 	return nil, ErrNotFound
 }
 
+// GetWithNewTTL retrieves an image and replaces its TTL with a new value.
+// The expiration time is set to now + newTtl.
+// Returns ErrNotFound if the key doesn't exist.
+// Thread-safe.
 func (i *ImageCache) GetWithNewTTL(key string, newTtl time.Duration) (image.Image, error) {
 	i.mu.Lock()
 	defer i.mu.Unlock()
@@ -132,6 +158,8 @@ func (i *ImageCache) GetWithNewTTL(key string, newTtl time.Duration) (image.Imag
 	return nil, ErrNotFound
 }
 
+// Clear removes all items from the cache.
+// Thread-safe.
 func (i *ImageCache) Clear() {
 	i.mu.Lock()
 	defer i.mu.Unlock()

backend/ipc/conn_other.go

@@ -11,6 +11,12 @@ import (
 	"runtime"
 )
 
+// socketPath is automatically initialized based on platform conventions:
+//   - macOS: ~/Library/Caches/supersonic/supersonic.sock (or /tmp/supersonic-{uid}.sock as fallback)
+//   - Linux/Unix: $XDG_RUNTIME_DIR/supersonic.sock (or /tmp/supersonic-{uid}.sock as fallback)
+//
+// TODO: Add support for portable mode by allowing override via environment variable
+// or configuration file (e.g., SUPERSONIC_SOCKET_PATH).
 var socketPath = "/tmp/supersonic.sock"
 
 func init() {
@@ -29,15 +35,21 @@ func init() {
 	}
 }
 
+// Dial establishes a connection to the IPC socket.
+// Returns an error if the socket doesn't exist or connection fails.
 func Dial() (net.Conn, error) {
-	// TODO - use XDG runtime dir, also handle portable mode
 	return net.Dial("unix", socketPath)
 }
 
+// Listen creates a Unix domain socket listener at the configured path.
+// The socket file is created automatically and should be cleaned up
+// with DestroyConn() when done.
 func Listen() (net.Listener, error) {
 	return net.Listen("unix", socketPath)
 }
 
+// DestroyConn removes the Unix socket file from the filesystem.
+// Should be called during application shutdown.
 func DestroyConn() error {
 	return os.Remove(socketPath)
 }

backend/util/stopwatch.go

@@ -1,14 +1,26 @@
 package util
 
-import "time"
+import (
+	"sync"
+	"time"
+)
 
+// Stopwatch is a thread-safe timer for measuring elapsed time.
+// It can be started, stopped, and reset, and supports reading
+// the elapsed time while running or stopped.
 type Stopwatch struct {
+	mu      sync.Mutex
 	running bool
 	started time.Time
 	elapsed time.Duration
 }
 
+// Start begins or resumes the stopwatch.
+// If already running, this is a no-op.
 func (s *Stopwatch) Start() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if s.running {
 		return
 	}
@@ -16,7 +28,12 @@ func (s *Stopwatch) Start() {
 	s.running = true
 }
 
+// Stop pauses the stopwatch and accumulates the elapsed time.
+// If already stopped, this is a no-op.
 func (s *Stopwatch) Stop() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	if !s.running {
 		return
 	}
@@ -24,7 +41,13 @@ func (s *Stopwatch) Stop() {
 	s.running = false
 }
 
+// Elapsed returns the total elapsed time.
+// If the stopwatch is running, includes time since last Start().
+// Safe to call concurrently with other methods.
 func (s *Stopwatch) Elapsed() time.Duration {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	e := s.elapsed
 	if s.running {
 		e += time.Since(s.started)
@@ -32,7 +55,11 @@ func (s *Stopwatch) Elapsed() time.Duration {
 	return e
 }
 
+// Reset stops the stopwatch and clears the elapsed time.
 func (s *Stopwatch) Reset() {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
 	s.running = false
 	s.elapsed = time.Duration(0)
 }

backend/util/stopwatch_test.go

@@ -0,0 +1,167 @@
+package util
+
+import (
+	"sync"
+	"testing"
+	"time"
+)
+
+func TestStopwatch_Basic(t *testing.T) {
+	sw := &Stopwatch{}
+
+	// Test initial state
+	if elapsed := sw.Elapsed(); elapsed != 0 {
+		t.Errorf("Expected initial elapsed time to be 0, got %v", elapsed)
+	}
+
+	// Test start and elapsed
+	sw.Start()
+	time.Sleep(10 * time.Millisecond)
+	elapsed := sw.Elapsed()
+	if elapsed < 10*time.Millisecond {
+		t.Errorf("Expected at least 10ms elapsed, got %v", elapsed)
+	}
+
+	// Test stop
+	sw.Stop()
+	stoppedElapsed := sw.Elapsed()
+	time.Sleep(10 * time.Millisecond)
+	if sw.Elapsed() != stoppedElapsed {
+		t.Error("Elapsed time should not increase after Stop()")
+	}
+
+	// Test reset
+	sw.Reset()
+	if elapsed := sw.Elapsed(); elapsed != 0 {
+		t.Errorf("Expected elapsed time to be 0 after reset, got %v", elapsed)
+	}
+}
+
+func TestStopwatch_StartStop(t *testing.T) {
+	sw := &Stopwatch{}
+
+	// Start, accumulate some time
+	sw.Start()
+	time.Sleep(10 * time.Millisecond)
+	sw.Stop()
+	firstElapsed := sw.Elapsed()
+
+	// Start again, accumulate more time
+	sw.Start()
+	time.Sleep(10 * time.Millisecond)
+	sw.Stop()
+	secondElapsed := sw.Elapsed()
+
+	if secondElapsed <= firstElapsed {
+		t.Errorf("Expected elapsed time to accumulate, first=%v second=%v", firstElapsed, secondElapsed)
+	}
+}
+
+func TestStopwatch_DoubleStart(t *testing.T) {
+	sw := &Stopwatch{}
+
+	sw.Start()
+	time.Sleep(5 * time.Millisecond)
+	firstStart := sw.Elapsed()
+
+	// Second Start() should be no-op
+	sw.Start()
+	time.Sleep(5 * time.Millisecond)
+	secondStart := sw.Elapsed()
+
+	// Time should continue from first start
+	if secondStart < firstStart {
+		t.Error("Second Start() affected timing")
+	}
+}
+
+func TestStopwatch_DoubleStop(t *testing.T) {
+	sw := &Stopwatch{}
+
+	sw.Start()
+	time.Sleep(10 * time.Millisecond)
+	sw.Stop()
+	elapsed := sw.Elapsed()
+
+	// Second Stop() should be no-op
+	sw.Stop()
+	if sw.Elapsed() != elapsed {
+		t.Error("Second Stop() changed elapsed time")
+	}
+}
+
+func TestStopwatch_ConcurrentAccess(t *testing.T) {
+	sw := &Stopwatch{}
+	var wg sync.WaitGroup
+
+	// Test concurrent Start/Stop/Elapsed calls
+	// This should not cause data races
+	const goroutines = 10
+	const iterations = 100
+
+	wg.Add(goroutines * 3)
+
+	// Concurrent starts
+	for i := 0; i < goroutines; i++ {
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations; j++ {
+				sw.Start()
+				time.Sleep(time.Microsecond)
+			}
+		}()
+	}
+
+	// Concurrent stops
+	for i := 0; i < goroutines; i++ {
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations; j++ {
+				sw.Stop()
+				time.Sleep(time.Microsecond)
+			}
+		}()
+	}
+
+	// Concurrent reads
+	for i := 0; i < goroutines; i++ {
+		go func() {
+			defer wg.Done()
+			for j := 0; j < iterations; j++ {
+				_ = sw.Elapsed()
+				time.Sleep(time.Microsecond)
+			}
+		}()
+	}
+
+	wg.Wait()
+
+	// If we got here without data races, the test passes
+	// Run with: go test -race
+}
+
+func TestStopwatch_Reset(t *testing.T) {
+	sw := &Stopwatch{}
+
+	// Reset when stopped
+	sw.Reset()
+	if elapsed := sw.Elapsed(); elapsed != 0 {
+		t.Errorf("Expected 0 after reset, got %v", elapsed)
+	}
+
+	// Reset when running
+	sw.Start()
+	time.Sleep(10 * time.Millisecond)
+	sw.Reset()
+	if elapsed := sw.Elapsed(); elapsed != 0 {
+		t.Errorf("Expected 0 after reset while running, got %v", elapsed)
+	}
+
+	// After reset, should be able to start again
+	sw.Start()
+	time.Sleep(5 * time.Millisecond)
+	elapsed := sw.Elapsed()
+	if elapsed < 5*time.Millisecond {
+		t.Errorf("Expected at least 5ms after reset and start, got %v", elapsed)
+	}
+}

backend/windows/notify.go

@@ -1,75 +1,75 @@
-//go:build windows
+//go:build windows
-
+
-package windows
+package windows
-
+
-import (
+import (
-	"fmt"
+	"fmt"
-	"os"
+	"os"
-	"os/exec"
+	"os/exec"
-	"path/filepath"
+	"path/filepath"
-	"strings"
+	"strings"
-	"syscall"
+	"syscall"
-
+
-	"fyne.io/fyne/v2"
+	"fyne.io/fyne/v2"
-)
+)
-
+
-// The general approach here is copied from Fyne.
+// The general approach here is copied from Fyne.
-// While it seems very hacky (create a temporary Powershell script and execute it),
+// While it seems very hacky (create a temporary Powershell script and execute it),
-// shockingly it may be the best approach, at least in the non-installed case.
+// shockingly it may be the best approach, at least in the non-installed case.
-// The proper Windows APIs for this require WinRT (ie C++/ a DLL), and also require
+// The proper Windows APIs for this require WinRT (ie C++/ a DLL), and also require
-// the app to be installed with a unique ID in the start menu, and to pass this ID
+// the app to be installed with a unique ID in the start menu, and to pass this ID
-// when sending the notification. This could be a future exploration for the installer.
+// when sending the notification. This could be a future exploration for the installer.
-
+
-const notificationTemplate = `$title = "%s"
+const notificationTemplate = `$title = "%s"
-$content = "%s"
+$content = "%s"
-$iconPath = "file:///%s"
+$iconPath = "file:///%s"
-[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null
+[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null
-$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastImageAndText02)
+$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastImageAndText02)
-$toastXml = [xml] $template.GetXml()
+$toastXml = [xml] $template.GetXml()
-$toastXml.GetElementsByTagName("text")[0].AppendChild($toastXml.CreateTextNode($title)) > $null
+$toastXml.GetElementsByTagName("text")[0].AppendChild($toastXml.CreateTextNode($title)) > $null
-$toastXml.GetElementsByTagName("text")[1].AppendChild($toastXml.CreateTextNode($content)) > $null
+$toastXml.GetElementsByTagName("text")[1].AppendChild($toastXml.CreateTextNode($content)) > $null
-$toastXml.GetElementsByTagName("image")[0].SetAttribute("src", $iconPath) > $null
+$toastXml.GetElementsByTagName("image")[0].SetAttribute("src", $iconPath) > $null
-$audio = $toastXml.CreateElement("audio")
+$audio = $toastXml.CreateElement("audio")
-$audio.SetAttribute("silent", "true") > $null
+$audio.SetAttribute("silent", "true") > $null
-$toastXml.DocumentElement.AppendChild($audio) > $null
+$toastXml.DocumentElement.AppendChild($audio) > $null
-$xml = New-Object Windows.Data.Xml.Dom.XmlDocument
+$xml = New-Object Windows.Data.Xml.Dom.XmlDocument
-$xml.LoadXml($toastXml.OuterXml)
+$xml.LoadXml($toastXml.OuterXml)
-$toast = [Windows.UI.Notifications.ToastNotification]::new($xml)
+$toast = [Windows.UI.Notifications.ToastNotification]::new($xml)
-[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("%s").Show($toast);`
+[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("%s").Show($toast);`
-
+
-func SendNotification(n *fyne.Notification, iconFilePath string) {
+func SendNotification(n *fyne.Notification, iconFilePath string) {
-	title := escapeNotificationString(n.Title)
+	title := escapeNotificationString(n.Title)
-	content := escapeNotificationString(n.Content)
+	content := escapeNotificationString(n.Content)
-
+
-	script := fmt.Sprintf(notificationTemplate, title, content, iconFilePath, "supersonic")
+	script := fmt.Sprintf(notificationTemplate, title, content, iconFilePath, "supersonic")
-	go runScript("notify", script)
+	go runScript("notify", script)
-}
+}
-
+
-func escapeNotificationString(in string) string {
+func escapeNotificationString(in string) string {
-	noSlash := strings.ReplaceAll(in, "`", "``")
+	noSlash := strings.ReplaceAll(in, "`", "``")
-	return strings.ReplaceAll(noSlash, "\"", "`\"")
+	return strings.ReplaceAll(noSlash, "\"", "`\"")
-}
+}
-
+
-var scriptNum = 0
+var scriptNum = 0
-
+
-func runScript(name, script string) {
+func runScript(name, script string) {
-	scriptNum++
+	scriptNum++
-	appID := fyne.CurrentApp().UniqueID()
+	appID := fyne.CurrentApp().UniqueID()
-	fileName := fmt.Sprintf("supersonic-%s-%s-%d.ps1", appID, name, scriptNum)
+	fileName := fmt.Sprintf("supersonic-%s-%s-%d.ps1", appID, name, scriptNum)
-
+
-	tmpFilePath := filepath.Join(os.TempDir(), fileName)
+	tmpFilePath := filepath.Join(os.TempDir(), fileName)
-	err := os.WriteFile(tmpFilePath, []byte(script), 0o600)
+	err := os.WriteFile(tmpFilePath, []byte(script), 0o600)
-	if err != nil {
+	if err != nil {
-		fyne.LogError("Could not write script to show notification", err)
+		fyne.LogError("Could not write script to show notification", err)
-		return
+		return
-	}
+	}
-	defer os.Remove(tmpFilePath)
+	defer os.Remove(tmpFilePath)
-
+
-	launch := "(Get-Content -Encoding UTF8 -Path " + tmpFilePath + " -Raw) | Invoke-Expression"
+	launch := "(Get-Content -Encoding UTF8 -Path " + tmpFilePath + " -Raw) | Invoke-Expression"
-	cmd := exec.Command("PowerShell", "-ExecutionPolicy", "Bypass", launch)
+	cmd := exec.Command("PowerShell", "-ExecutionPolicy", "Bypass", launch)
-	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
-	err = cmd.Run()
+	err = cmd.Run()
-	if err != nil {
+	if err != nil {
-		fyne.LogError("Failed to launch windows notify script", err)
+		fyne.LogError("Failed to launch windows notify script", err)
-	}
+	}
-}
+}

backend/windows/notify_unsupported.go

@@ -1,10 +1,10 @@
-//go:build !windows
+//go:build !windows
-
+
-package windows
+package windows
-
+
-import "fyne.io/fyne/v2"
+import "fyne.io/fyne/v2"
-
+
-func SendNotification(n *fyne.Notification, iconFilePath string) {
+func SendNotification(n *fyne.Notification, iconFilePath string) {
-	fyne.LogError("windows.SendNotification should not be invoked on non-Windows platform", nil)
+	fyne.LogError("windows.SendNotification should not be invoked on non-Windows platform", nil)
-	fyne.CurrentApp().SendNotification(n)
+	fyne.CurrentApp().SendNotification(n)
-}
+}