From 4903d643561bb9fff1add6452f97c54562930e32 Mon Sep 17 00:00:00 2001
From: Graham Clark <grclark@gmail.com>
Date: Sat, 4 Jun 2022 11:05:29 -0400
Subject: [PATCH] Fix a bug that could cause a hang at shutdown

The new search widget is created during ui.Build(), which always
runs. That means the search widget needs to be closed, whether termshark
starts successfully or not. During normal operation, with the UI
running, a message is sent to a "quit" channel and the main select{}
loop in termshark.go will call Close() on the widgets with extra
goroutines and other resources. I included the search widget in that
list. But if termshark doesn't get to the point of the UI running, these
Close() functions are not called. For the cases I have, it
doesn't *really* matter because these would just leave dangling
goroutines at shutdown - no harm done. But termshark tries to clean up
every resource it creates just as matter of good practice, and if
goroutines are left unterminated, the main process will deliberately
hang at shutdown (so I can detect that).

Termshark can bail out after the UI creation but before the UI launches
if e.g. the launching user does not have permission to sniff on an
interface:

foobar@elgin:~$ /home/gcla/go/bin/termshark --debug -i 3
(The termshark UI will start when packets are detected on Random packet generator...)
Cannot capture on device Random packet generator: exit status 1 (exit code 1)
You might need: sudo setcap cap_net_raw,cap_net_admin+eip /home/gcla/go/bin/termshark
Or try running with sudo or as root.
See https://termshark.io/no-root for more info.
---
 cmd/termshark/termshark.go | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/cmd/termshark/termshark.go b/cmd/termshark/termshark.go
index f5c4ee0..ce80f49 100644
--- a/cmd/termshark/termshark.go
+++ b/cmd/termshark/termshark.go
@@ -816,6 +816,9 @@ func cmain() int {
 		}
 	}
 
+	// the app variable is created here so I can bind it in the defer below
+	var app *gowid.App
+
 	// Do this before ui.Build. If ui.Build fails (e.g. bad TERM), then the filter will be left
 	// running, so we need the defer to be in effect here and not after the processing of ui.Build's
 	// error
@@ -823,9 +826,17 @@ func cmain() int {
 		if ui.FilterWidget != nil {
 			ui.FilterWidget.Close()
 		}
+		if ui.SearchWidget != nil {
+			ui.SearchWidget.Close(app)
+		}
+		if ui.CurrentWormholeWidget != nil {
+			ui.CurrentWormholeWidget.Close()
+		}
+		if ui.CurrentColsWidget != nil {
+			ui.CurrentColsWidget.Close()
+		}
 	}()
 
-	var app *gowid.App
 	if app, err = ui.Build(); err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		// Tcell returns ExitError now because if its internal terminfo DB does not have
@@ -1235,15 +1246,6 @@ Loop:
 				// will happen next time round because the quitRequested flag is checked.
 				stopLoaders()
 			}
-			if ui.CurrentWormholeWidget != nil {
-				ui.CurrentWormholeWidget.Close()
-			}
-			if ui.CurrentColsWidget != nil {
-				ui.CurrentColsWidget.Close()
-			}
-			if ui.SearchWidget != nil {
-				ui.SearchWidget.Close(app)
-			}
 
 		case sig := <-sigChan:
 			if system.IsSigTSTP(sig) {