Skip to content

Commit

Permalink
Fix a bug that could cause a hang at shutdown
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gcla committed Jun 4, 2022
1 parent f087f72 commit 4903d64
Showing 1 changed file with 12 additions and 10 deletions.
22 changes: 12 additions & 10 deletions cmd/termshark/termshark.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,16 +816,27 @@ 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
defer func() {
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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 4903d64

Please sign in to comment.