PagerDuty / go-pagerduty

go client library for PagerDuty v2 API
https://v2.developer.pagerduty.com/docs/rest-api
Apache License 2.0
287 stars 240 forks source link

Fix memory leak #453

Closed attilakunelwood closed 2 years ago

attilakunelwood commented 2 years ago

This commit fixes a memory leak that happens if the PagerDuty endpoint returns a 429 HTTP status code. It was not verified whether the error happens with other status codes as well.

I verified the correctness of this fix by running the following test:

func TestMemoryLeak(t *testing.T) {
    evt := &pagerduty.V2Event{
        RoutingKey: routingKey,
        Action:     "trigger",
        Payload: &pagerduty.V2Payload{
            Summary:   "foo",
            Source:    "bar",
            Component: serviceName,
            Severity:  "critical",
            Class:     "",
            Details:   nil,
        },
    }

    for i := 0; i < 100; i++ {
        client.ManageEvent(evt)
        fmt.Printf("num goroutines: %d\n", runtime.NumGoroutine())
        time.Sleep(time.Millisecond * 50)
    }
}

Without the fix, this produces the following output:

num goroutines: 6
num goroutines: 8
num goroutines: 10
num goroutines: 12
num goroutines: 14
num goroutines: 16
num goroutines: 18
num goroutines: 20
num goroutines: 22
num goroutines: 24
...

With the fix, this produces the following output:

num goroutines: 8
num goroutines: 8
num goroutines: 8
num goroutines: 8
num goroutines: 8
num goroutines: 8
num goroutines: 6
num goroutines: 6
num goroutines: 6
num goroutines: 6
num goroutines: 5
num goroutines: 6
...
ChuckCrawford commented 2 years ago

Yeah thank you for the fix @attilakunelwood! We are going to have one more look at this and see if there isn't a more centralized location for us to put this fix.