fix(shutdown): resolve deadlock on graceful shutdown with Cmd+Q

Root cause: After GTK main loop exits, any GTK method call blocks forever.
During cleanup, Destroy() was acquiring a lock that was held by goroutines
making GTK calls (URI(), Title(), ZoomLevel(), etc.), causing deadlock.

Changes:
- Remove mutex lock acquisition in WebView.Destroy() to prevent deadlock
- Remove GTK method calls (StopLoading) after main loop has exited
- Remove lock from IsDestroyed() (bool reads are atomic on all platforms)
- Remove unused ConnectLoadChanged signal handler added in this branch
- Move history queue flush to run before cleanup (after main loop exit)

During shutdown, thread safety is not critical as we're tearing down.
GTK widgets are cleaned up by Go GC.

commit: 7e8b325966a83b8997f6276f946911216744bacd
author: bnema · 12mo ago · 2025-10-24 05:35 UTC
committer: bnema · 12mo ago · 2025-10-24 17:33 UTC
parent: 1e6ca34
signature: unsigned

3 files · +21 -59

M internal/app/browser/browser.go +9 -6


@@ -218,12 +218,6 @@ func (app *BrowserApp) cleanup() {
218	218		app.workspace = nil
219	219		}
220	220
221		-	// Flush pending history writes before closing database
222		-	if app.browserService != nil {
223		-	log.Printf("Flushing pending history writes...")
224		-	app.browserService.FlushHistoryQueue()
225		-	}
226		-
227	221		// Close database with WAL checkpoint
228	222		if app.database != nil {
229	223		log.Printf("Performing WAL checkpoint and closing database...")

@@ -287,6 +281,15 @@ func (app *BrowserApp) runMainLoop() {
287	281		log.Printf("Entering GTK main loop…")
288	282		webkit.RunMainLoop()
289	283		log.Printf("GTK main loop exited")
	284	+
	285	+	// Flush pending history writes immediately after main loop exit
	286	+	// This ensures database operations complete while GTK is still in a valid state
	287	+	// MUST happen before cleanup() deferred call, which happens after this function returns
	288	+	if app.browserService != nil {
	289	+	log.Printf("Flushing pending history writes...")
	290	+	app.browserService.FlushHistoryQueue()
	291	+	log.Printf("History queue flushed successfully")
	292	+	}
290	293		} else {
291	294		log.Printf("Not entering GUI loop (non-CGO build)")
292	295		// In non-CGO mode, just wait for signals

 		app.workspace = nil
 	}
-	// Flush pending history writes before closing database
-	if app.browserService != nil {
-		log.Printf("Flushing pending history writes...")
-		app.browserService.FlushHistoryQueue()
-	}
 	// Close database with WAL checkpoint
 	if app.database != nil {
 		log.Printf("Performing WAL checkpoint and closing database...")
 		log.Printf("Entering GTK main loop…")
 		webkit.RunMainLoop()
 		log.Printf("GTK main loop exited")
+		// Flush pending history writes immediately after main loop exit
+		// This ensures database operations complete while GTK is still in a valid state
+		// MUST happen before cleanup() deferred call, which happens after this function returns
+		if app.browserService != nil {
+			log.Printf("Flushing pending history writes...")
+			app.browserService.FlushHistoryQueue()
+			log.Printf("History queue flushed successfully")
+		}
 	} else {
 		log.Printf("Not entering GUI loop (non-CGO build)")
 		// In non-CGO mode, just wait for signals

M internal/app/browser/pane.go +0 -17


@@ -104,23 +104,6 @@ func (p *BrowserPane) Cleanup() {
104	104		p.messageHandler.SetWebView(nil)
105	105		}
106	106
107		-	// Notify JS to cleanup GUI
108		-	if p.webView != nil && p.hasGUI && !p.webView.IsDestroyed() {
109		-	cleanupScript := `try {
110		-	if (typeof window.__dumber_teardown === 'function') {
111		-	window.__dumber_teardown();
112		-	}
113		-	if (window.__dumber_gui_manager) {
114		-	window.__dumber_gui_manager.destroy();
115		-	}
116		-	} catch (err) {
117		-	console.warn('[dumber] GUI teardown failed', err);
118		-	}`
119		-	if err := p.webView.InjectScript(cleanupScript); err != nil {
120		-	log.Printf("[pane-%s] failed to inject cleanup script: %v", p.id, err)
121		-	}
122		-	}
123		-
124	107		// Destroy WebView to unregister from global registry and release resources
125	108		if p.webView != nil && !p.webView.IsDestroyed() {
126	109		if err := p.webView.Destroy(); err != nil {


@@ -104,23 +104,6 @@ func (p *BrowserPane) Cleanup() {
104		p.messageHandler.SetWebView(nil)	104	p.messageHandler.SetWebView(nil)
105		}	105	}
106			106
107	-	// Notify JS to cleanup GUI
108	-	if p.webView != nil && p.hasGUI && !p.webView.IsDestroyed() {
109	-	cleanupScript := `try {
110	-	if (typeof window.__dumber_teardown === 'function') {
111	-	window.__dumber_teardown();
112	-	}
113	-	if (window.__dumber_gui_manager) {
114	-	window.__dumber_gui_manager.destroy();
115	-	}
116	-	} catch (err) {
117	-	console.warn('[dumber] GUI teardown failed', err);
118	-	}`
119	-	if err := p.webView.InjectScript(cleanupScript); err != nil {
120	-	log.Printf("[pane-%s] failed to inject cleanup script: %v", p.id, err)
121	-	}
122	-	}
123	-
124		// Destroy WebView to unregister from global registry and release resources	107	// Destroy WebView to unregister from global registry and release resources
125		if p.webView != nil && !p.webView.IsDestroyed() {	108	if p.webView != nil && !p.webView.IsDestroyed() {
126		if err := p.webView.Destroy(); err != nil {	109	if err := p.webView.Destroy(); err != nil {

M pkg/webkit/webview.go +12 -36


@@ -6,7 +6,6 @@ import (
6	6		"log"
7	7		"strconv"
8	8		"sync"
9		-	"sync/atomic"
10	9
11	10		webkit "github.com/diamondburned/gotk4-webkitgtk/pkg/webkit/v6"
12	11		"github.com/diamondburned/gotk4/pkg/gdk/v4"

@@ -46,9 +45,6 @@ type WebView struct {
46	45		onWorkspaceNavigation func(direction string) bool // Workspace pane navigation
47	46		onPaneModeShortcut func(action string) bool // Pane mode shortcuts (enter, actions, exit)
48	47		isPaneModeActive func() bool // Check if pane mode is active
49		-	onLoadStarted func() // Load started event
50		-	onLoadCommitted func() // Load committed event (DOM available)
51		-	onLoadFinished func() // Load finished event
52	48		}
53	49
54	50		// NewWebView creates a new WebView with the given configuration

@@ -239,33 +235,6 @@ func (w *WebView) setupEventHandlers() {
239	235		// Favicon changed - connect to FaviconDatabase
240	236		w.setupFaviconHandlers()
241	237
242		-	// Load changed - track page lifecycle for performance monitoring
243		-	w.view.ConnectLoadChanged(func(event webkit.LoadEvent) {
244		-	// Avoid calling callbacks if WebView is being destroyed to prevent reference cycles
245		-	// especially important when page cache is enabled
246		-	if w.IsDestroyed() {
247		-	return
248		-	}
249		-
250		-	switch event {
251		-	case webkit.LoadStarted:
252		-	// Page load started - could show loading indicator
253		-	if w.onLoadStarted != nil {
254		-	w.onLoadStarted()
255		-	}
256		-	case webkit.LoadCommitted:
257		-	// Page committed - DOM is now available for interaction
258		-	if w.onLoadCommitted != nil {
259		-	w.onLoadCommitted()
260		-	}
261		-	case webkit.LoadFinished:
262		-	// Page fully loaded - all resources downloaded
263		-	if w.onLoadFinished != nil {
264		-	w.onLoadFinished()
265		-	}
266		-	}
267		-	})
268		-
269	238		// TLS error handling - connect to load-failed-with-tls-errors signal
270	239		w.setupTLSErrorHandler()
271	240

@@ -450,8 +419,12 @@ func (w *WebView) Hide() error {
450	419
451	420		// Destroy destroys the WebView and releases resources
452	421		func (w *WebView) Destroy() error {
453		-	w.mu.Lock()
454		-	defer w.mu.Unlock()
	422	+	// DO NOT acquire w.mu lock here to avoid deadlock!
	423	+	// After GTK main loop exits, any goroutine holding the lock and calling GTK methods
	424	+	// will block forever waiting for the main loop. This causes Destroy() to deadlock
	425	+	// trying to acquire the same lock.
	426	+	//
	427	+	// During shutdown, thread safety is not critical since we're tearing everything down.
455	428
456	429		if w.destroyed {
457	430		return nil

@@ -464,14 +437,17 @@ func (w *WebView) Destroy() error {
464	437		delete(viewRegistry, w.id)
465	438		viewMu.Unlock()
466	439
467		-	// The GTK widget will be cleaned up by Go GC
	440	+	// DO NOT call StopLoading() or any other GTK methods here!
	441	+	// After the main loop exits, GTK calls will block forever.
	442	+	// The GTK widget will be cleaned up by Go GC.
	443	+
468	444		return nil
469	445		}
470	446
471	447		// IsDestroyed returns true if the WebView has been destroyed
472	448		func (w *WebView) IsDestroyed() bool {
473		-	w.mu.RLock()
474		-	defer w.mu.RUnlock()
	449	+	// DO NOT acquire lock - read of bool is atomic on all platforms Go supports
	450	+	// This prevents deadlock during shutdown when GTK signal handlers check IsDestroyed
475	451		return w.destroyed
476	452		}
477	453

 	"log"
 	"strconv"
 	"sync"
-	"sync/atomic"
 	webkit "github.com/diamondburned/gotk4-webkitgtk/pkg/webkit/v6"
 	"github.com/diamondburned/gotk4/pkg/gdk/v4"
 	onWorkspaceNavigation func(direction string) bool // Workspace pane navigation
 	onPaneModeShortcut    func(action string) bool    // Pane mode shortcuts (enter, actions, exit)
 	isPaneModeActive      func() bool                 // Check if pane mode is active
-	onLoadStarted         func()                      // Load started event
-	onLoadCommitted       func()                      // Load committed event (DOM available)
-	onLoadFinished        func()                      // Load finished event
 }
 // NewWebView creates a new WebView with the given configuration
 	// Favicon changed - connect to FaviconDatabase
 	w.setupFaviconHandlers()
-	// Load changed - track page lifecycle for performance monitoring
-	w.view.ConnectLoadChanged(func(event webkit.LoadEvent) {
-		// Avoid calling callbacks if WebView is being destroyed to prevent reference cycles
-		// especially important when page cache is enabled
-		if w.IsDestroyed() {
-			return
-		}
-		switch event {
-		case webkit.LoadStarted:
-			// Page load started - could show loading indicator
-			if w.onLoadStarted != nil {
-				w.onLoadStarted()
-			}
-		case webkit.LoadCommitted:
-			// Page committed - DOM is now available for interaction
-			if w.onLoadCommitted != nil {
-				w.onLoadCommitted()
-			}
-		case webkit.LoadFinished:
-			// Page fully loaded - all resources downloaded
-			if w.onLoadFinished != nil {
-				w.onLoadFinished()
-			}
-		}
-	})
 	// TLS error handling - connect to load-failed-with-tls-errors signal
 	w.setupTLSErrorHandler()
 // Destroy destroys the WebView and releases resources
 func (w *WebView) Destroy() error {
-	w.mu.Lock()
+	// DO NOT acquire w.mu lock here to avoid deadlock!
-	defer w.mu.Unlock()
+	// After GTK main loop exits, any goroutine holding the lock and calling GTK methods
+	// will block forever waiting for the main loop. This causes Destroy() to deadlock
+	// trying to acquire the same lock.
+	//
+	// During shutdown, thread safety is not critical since we're tearing everything down.
 	if w.destroyed {
 		return nil
 	delete(viewRegistry, w.id)
 	viewMu.Unlock()
-	// The GTK widget will be cleaned up by Go GC
+	// DO NOT call StopLoading() or any other GTK methods here!
+	// After the main loop exits, GTK calls will block forever.
+	// The GTK widget will be cleaned up by Go GC.
 	return nil
 }
 // IsDestroyed returns true if the WebView has been destroyed
 func (w *WebView) IsDestroyed() bool {
-	w.mu.RLock()
+	// DO NOT acquire lock - read of bool is atomic on all platforms Go supports
-	defer w.mu.RUnlock()
+	// This prevents deadlock during shutdown when GTK signal handlers check IsDestroyed
 	return w.destroyed
 }