uber-go / goleak

Goroutine leak detector
MIT License
4.48k stars 148 forks source link

internal/stack: Use control flow for state #110

Closed abhinav closed 10 months ago

abhinav commented 10 months ago

In anticipation of parsing more information from stack traces make the stack trace parsing logic more manageable by moving it from a state machine into a layout closer to a recursive descent parser.

That is, instead of a central loop that reads input line-by-line and needs to manage its various states:

current, result := ...
for {
    input := read()
    if cond(input) {
        result.append(current)
        current = startNew(input)
    } else {
        current = accumulate(input)
    }
}
result = flush(current)

Break it down so that parsing of individual results is its own function, representing the state machine via control flow.

result := ...
for {
    input := read()
    if cond(input) {
        result.append(parseOne())
    }
}

// where

func parseOne(input) {
    value := ...
    for ; !cond(input); input = read() {
        value = accumulate(input)
    }
    return value
}

The net effect of this is to make the parsing logic more maintainable once it gets more complex -- adds more states.

For example, to parse more information for individual stacks with a state machine, we'd have to make the main loop more complex. State for an individual stack (e.g. "all the functions in the stack") will leak into the state management for the whole state machine. On the other hand, with this method, we'll only modify parseStack, keeping its responsiblity encapsulated to parsing a single stack trace.

This idea was also demonstrated recently in the first section of Storing Data in Control flow by Russ Cox.


To make it easy to write this parser, we switch from bufio.Reader to bufio.Scanner, and wrap it with the ability to "Unscan": basically "don't move forward on next Scan()".

Lastly, we need to bump the go directive in go.mod to Go 1.20 to allow use of errors.Join.

codecov[bot] commented 10 months ago

Codecov Report

Merging #110 (aee45b6) into master (f995fdb) will increase coverage by 1.60%. The diff coverage is 93.50%.

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
+ Coverage   96.58%   98.18%   +1.60%     
==========================================
  Files           5        6       +1     
  Lines         234      276      +42     
==========================================
+ Hits          226      271      +45     
+ Misses          5        4       -1     
+ Partials        3        1       -2     
Files Coverage Δ
internal/stack/scan.go 100.00% <100.00%> (ø)
internal/stack/stacks.go 95.14% <92.53%> (+6.41%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more