bugfix: deadline.Set() memory leak (issue #208)

[floatingstatic]

Signed-off-by: Jeremiah Millay jmillay@fastly.com

Description

Use time.Newtimer() paired with timer.Stop() to avoid leaking memory with time.After() whose underlying timer will not be released to the garbage collector until it has fired.

Reference issue

Fixes #208

[codecov]

Codecov Report

Base: 84.39% // Head: 84.41% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (153a9ec) compared to base (4ce527f).
Patch coverage: 40.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #209      +/-   ##
==========================================
+ Coverage   84.39%   84.41%   +0.02%     
==========================================
  Files          31       31              
  Lines        2691     2695       +4     
==========================================
+ Hits         2271     2275       +4     
  Misses        315      315              
  Partials      105      105              

Flag	Coverage Δ
go	84.14% <40.00%> (-0.09%)	⬇️
wasm	77.47% <40.00%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
deadline/deadline.go	88.40% <40.00%> (-3.91%)	⬇️
vnet/delay_filter.go	92.68% <0.00%> (+7.31%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jech]

if !timer.Stop() {
    <-timer.C
}

Why is that needed? I'd just say timer.Stop() and be done with it.

Other than that, the patch looks correct to me.

[floatingstatic]

@jech this is the documented best practice for draining the timer's channel after calling stop. See here:

https://pkg.go.dev/time#Timer.Stop

[jech]

@jech this is the documented best practice for draining the timer's channel after calling stop. See here: https://pkg.go.dev/time#Timer.Stop

Ah, in case the timer is shared between goroutines. No objection, then.

[floatingstatic]

@jech do you need me to do anything further here? Looks like the CI workflow is stuck pending approval.

[jech]

You need approval from one of the Pion members.

[Antonito]

Hi @floatingstatic,

looks good to me! Thank you for fixing this!

[stv0g]

This looks also good to me 👍