Skip to content

Commit

Permalink
First attempt to fix loader bug that means termshark doesn't quit
Browse files Browse the repository at this point in the history
This is an attempt to solve issue #121. I think my mistake is as
follows:

- I start the goroutine that runs the pcap process
- after starting the process, I send its pid via a channel to
  another goroutine that controls process life-cycles
- however that goroutine has closed down because the user issued
  a cancel

The process lifetime goroutine should not shut down if it is guaranteed
the pcap running goroutine is going to send its pid, because then
nothing will listen for it. I've adjusted the logic so now that
goroutine will definitely wait for the pid - unless it receives a pid==0
which indicates there was a problem starting the process.
  • Loading branch information
gcla committed Apr 12, 2021
1 parent 88f0374 commit 43216e2
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 9 deletions.
2 changes: 1 addition & 1 deletion capinfo/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (c *Loader) loadCapinfoAsync(pcapf string, app gowid.IApp, cb ICapinfoCallb
}
}

if state == pcap.Terminated || (cancelledChan == nil && state == pcap.NotStarted) {
if state == pcap.Terminated || (procChan == nil && state == pcap.NotStarted) {
break loop
}
}
Expand Down
2 changes: 1 addition & 1 deletion convs/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (c *Loader) loadConvAsync(pcapf string, convs []string, filter string, abs
}
}

if state == pcap.Terminated || (cancelledChan == nil && state == pcap.NotStarted) {
if state == pcap.Terminated || (procChan == nil && state == pcap.NotStarted) {
break loop
}
}
Expand Down
19 changes: 14 additions & 5 deletions pcap/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -1015,8 +1015,17 @@ func (c *PdmlLoader) loadPcapSync(row int, visible bool, ps iPdmlLoaderEnv, cb i
// a message means the proc has started
// closed means it won't be started
// if closed, then pdmlCmd == nil
if (pdmlState == Terminated || (pdmlCancelledChan == nil && pdmlState == NotStarted)) &&
(pcapState == Terminated || (pcapCancelledChan == nil && pcapState == NotStarted)) {
// 04/11/21: I can't take a shortcut here and condition on Terminated || (cancelledChan == nil && NotStarted)
// See the pcap or pdml goroutines below. I block at the beginning, checking on the stage2 cancellation.
// If I get past that point, and there are no errors in the process invocation, I am guaranteed to start both
// the pdml and pcap processes. If there are errors, I am guaranteed to close the pcapPidChan with a defer.
// If I take a shortcut and end this goroutine via a stage2 cancellation before waiting for the pcap pid,
// then I'll block in that goroutine, trying to send to the pcapPidChan, but with nothing here to receive
// the value. In the pcap process goroutine, if I get past the stage2 cancellation check, then I need to
// have something to receive the pid - this goroutine. It needs to stay alive until it gets the pid, or a
// zero.
if (pdmlState == Terminated || (pdmlPidChan == nil && c.PdmlPid == 0)) &&
(pcapState == Terminated || (pcapPidChan == nil && c.PcapPid == 0)) {
// nothing to select on so break
break loop
}
Expand Down Expand Up @@ -1464,7 +1473,7 @@ func (p *PsmlLoader) loadPsmlSync(iloader *InterfaceLoader, e iPsmlLoaderEnv, cb
}
}

if state == Terminated || (intCancelledChan == nil && state == NotStarted) {
if state == Terminated || (pidChan == nil && state == NotStarted) {
break loop
}
}
Expand Down Expand Up @@ -1590,7 +1599,7 @@ func (p *PsmlLoader) loadPsmlSync(iloader *InterfaceLoader, e iPsmlLoaderEnv, cb

// successfully started then died/kill, OR
// was never started, won't be started, and cancelled
if state == Terminated || (cancelledChan == nil && state == NotStarted) {
if state == Terminated || (pidChan == nil && state == NotStarted) {
break loop
}
}
Expand Down Expand Up @@ -2082,7 +2091,7 @@ func (i *InterfaceLoader) loadIfacesSync(e iIfaceLoaderEnv, cb interface{}, app
// a message means the proc has started
// closed means it won't be started
// if closed, then pdmlCmd == nil
if state == Terminated || (cancelledChan == nil && state == NotStarted) {
if state == Terminated || (pidChan == nil && state == NotStarted) {
// nothing to select on so break
break loop
}
Expand Down
4 changes: 2 additions & 2 deletions streams/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (c *Loader) loadStreamReassemblyAsync(pcapf string, proto string, idx int,
}
}

if state == pcap.Terminated || (cancelled == nil && state == pcap.NotStarted) {
if state == pcap.Terminated || (procChan == nil && state == pcap.NotStarted) {
break loop
}
}
Expand Down Expand Up @@ -300,7 +300,7 @@ func (c *Loader) startStreamIndexerAsync(pcapf string, proto string, idx int, ap

}

if state == pcap.Terminated || (cancelledChan == nil && state == pcap.NotStarted) {
if state == pcap.Terminated || (procChan == nil && state == pcap.NotStarted) {
break loop
}

Expand Down

0 comments on commit 43216e2

Please sign in to comment.