Skip to content

Commit

Permalink
Fix bug that resulted in sporadic failed stream reassembly
Browse files Browse the repository at this point in the history
The failure coincided with this error in the log:

msg="Stream parser reported error: read |0: file already closed"

The StdoutPipe docs explain the problem:
https://pkg.go.dev/os/exec?utm_source=godoc#Cmd.StdoutPipe

"Wait will close the pipe after seeing the command exit, so most callers
need not close the pipe themselves. It is thus incorrect to call Wait
before all reads from the pipe have completed. For the same reason, it
is incorrect to call Run when using StdoutPipe. See the example for
idiomatic usage."

My bug was that I Wait()ed for the stream process to end in a goroutine
while at the same time, I handed off the process's stdout to a parser to
build the stream data structures. Depending on timing, I might call
Wait() before the stream parser has finished reading the output - the
problem described above.
  • Loading branch information
gcla committed Jul 11, 2022
1 parent 63f62c3 commit e8de021
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

### Changed

- Fixed a bug that resulted in stream reassembly sporadically failing and displaying a blank screen.
- Termshark will now, by default, suppress errors from tshark. You can change this via the minibuffer
`set suppress-tshark-errors` command.
- Added a summary of standard error to the error dialogs displayed when a tshark process fails to run
Expand Down
6 changes: 3 additions & 3 deletions pkg/streams/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ func (c *Loader) loadStreamReassemblyAsync(pcapf string, proto string, idx int,

log.Infof("Started stream reassembly command %v with pid %d", c.streamCmd, c.streamCmd.Pid())

termshark.TrackedGo(func() {
defer func() {
termChan <- c.streamCmd.Wait()
}, Goroutinewg)
}()

pid = c.streamCmd.Pid()
procChan <- pid
Expand All @@ -219,7 +219,7 @@ func (c *Loader) loadStreamReassemblyAsync(pcapf string, proto string, idx int,
func() {
_, err := ParseReader("", streamOut, ops...)
if err != nil {
log.Infof("Stream parser reported error: %v", err)
log.Warnf("Stream parser reported error: %v", err)
}
}()

Expand Down

0 comments on commit e8de021

Please sign in to comment.