fix: avoid memory leak and proper cleanups (#80)

Author: y3owk1n

README.md

@@ -63,7 +63,6 @@ This is an intentional design choice to keep the project lean, maintainable, and
 - Consider zIndex for hints on different layers? There might be multiple layers when we are considering `Menubar`, `Dock` and `Notification bar`
 - Better UI representation for action menu (maybe auto edge detection like tooltip in browser, that will place itself around the element based on the space available around it)
 - Find a way to auto deduplicate hints that are targeting the same point
-- Scroll areas are hardcoded from 1-9, do we ever need more than that and make it dynamic upon query? We can still use <Tab> to cycle next tho
 - Get rid of `PageHeight` in config and find a way to calculate available scroll for (c-d, c-u, gg, G) more reliably.
 - Homerow supports `continuous clicks`, is that something important?
 - Cant actually make scroll works in Electron apps, what is the best approach? There are tons of random `AXGroup` where some is scrollable and some is not...

cmd/neru/main.go

@@ -1085,25 +1085,44 @@ func isLetter(b byte) bool {
 func (a *App) Cleanup() {
 	a.logger.Info("Cleaning up")
 
+	// Exit any active mode first
 	a.exitMode()
 
-	// Unregister all hotkeys
-	a.hotkeyManager.UnregisterAll()
-
-	a.hintOverlay.Destroy()
-
-	// Stop IPC server
+	// Stop IPC server first to prevent new requests
 	if a.ipcServer != nil {
 		if err := a.ipcServer.Stop(); err != nil {
 			a.logger.Error("Failed to stop IPC server", zap.Error(err))
 		}
 	}
 
+	// Unregister all hotkeys
+	if a.hotkeyManager != nil {
+		a.hotkeyManager.UnregisterAll()
+	}
+
+	// Clean up UI elements
+	if a.hintOverlay != nil {
+		a.hintOverlay.Destroy()
+	}
+
+	// Clean up scroll controller if exists
+	if a.scrollController != nil {
+		a.scrollController.Cleanup()
+	}
+
 	// Cleanup event tap
 	if a.eventTap != nil {
 		a.eventTap.Destroy()
 	}
 
+	// Sync and close logger at the end
+	if err := logger.Sync(); err != nil {
+		// Ignore "inappropriate ioctl for device" error which occurs when syncing stdout/stderr
+		if !strings.Contains(err.Error(), "inappropriate ioctl for device") {
+			a.logger.Error("Failed to sync logger", zap.Error(err))
+		}
+	}
+
 	// Stop app watcher
 	a.appWatcher.Stop()
 

internal/accessibility/cache.go

@@ -97,9 +97,6 @@ func (c *InfoCache) cleanup() {
 
 // Stop stops the cleanup goroutine
 func (c *InfoCache) Stop() {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
 	if !c.stopped {
 		close(c.stopCh)
 		c.stopped = true
@@ -110,14 +107,12 @@ func (c *InfoCache) Stop() {
 func (c *InfoCache) Clear() {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-
 	c.data = make(map[uintptr]*CachedInfo, 100)
 }
 
 // Size returns the number of cached entries
 func (c *InfoCache) Size() int {
 	c.mu.RLock()
 	defer c.mu.RUnlock()
-
 	return len(c.data)
 }

internal/accessibility/element.go

@@ -355,6 +355,15 @@ func (e *Element) Release() {
 	}
 }
 
+// ReleaseAll releases all elements in a slice
+func ReleaseAll(elements []*Element) {
+	for _, elem := range elements {
+		if elem != nil {
+			elem.Release()
+		}
+	}
+}
+
 // GetAllWindows returns all windows of the focused application
 func GetAllWindows() ([]*Element, error) {
 	var count C.int

internal/eventtap/eventtap.go

@@ -90,9 +90,16 @@ func (et *EventTap) Disable() {
 // Destroy destroys the event tap
 func (et *EventTap) Destroy() {
 	if et.handle != nil {
+		// Disable first to prevent any pending callbacks
+		et.Disable()
+
+		// Destroy the tap
 		C.destroyEventTap(et.handle)
 		et.handle = nil
 
+		// Clear callback to prevent any lingering references
+		et.callback = nil
+
 		// Clear global reference if this is the global event tap
 		globalEventTapMu.Lock()
 		if globalEventTap == et {

internal/ipc/ipc.go

@@ -105,6 +105,12 @@ func (s *Server) handleConnection(conn net.Conn) {
 		}
 	}()
 
+	// Set read deadline to prevent hanging connections
+	if err := conn.SetDeadline(time.Now().Add(30 * time.Second)); err != nil {
+		s.logger.Error("Failed to set connection deadline", zap.Error(err))
+		return
+	}
+
 	decoder := json.NewDecoder(conn)
 	encoder := json.NewEncoder(conn)
 

internal/logger/logger.go

@@ -117,20 +117,20 @@ func Close() error {
 	logFileMu.Lock()
 	defer logFileMu.Unlock()
 
-	// Sync logger first
 	if globalLogger != nil {
 		if err := globalLogger.Sync(); err != nil {
-			// Ignore sync errors on stdout/stderr (common on macOS)
-			// but log them for debugging
-			if !strings.Contains(err.Error(), "sync /dev/stdout") &&
-				!strings.Contains(err.Error(), "sync /dev/stderr") {
-				fmt.Fprintf(os.Stderr, "Warning: failed to sync logger: %v\n", err)
+			// Ignore common sync errors that occur during shutdown
+			if !strings.Contains(err.Error(), "invalid argument") &&
+				!strings.Contains(err.Error(), "inappropriate ioctl for device") {
+				return fmt.Errorf("failed to sync logger: %w", err)
 			}
 		}
+		globalLogger = nil
 	}
 
-	// Close log file
 	if logFile != nil {
+		// Best effort sync, ignore errors on stdout/stderr
+		_ = logFile.Sync()
 		if err := logFile.Close(); err != nil {
 			return fmt.Errorf("failed to close log file: %w", err)
 		}