client: avoid unconsumed channel in timer construction (#15215)

* client: avoid unconsumed channel in timer construction

This PR fixes a bug introduced in #11983 where a Timer initialized with 0
duration causes an immediate tick, even if Reset is called before reading the
channel. The fix is to avoid doing that, instead creating a Timer with a non-zero
initial wait time, and then immediately calling Stop.

* pr: remove redundant stop

.changelog/15215.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+client: Fixed a bug where tasks would restart without waiting for interval
+```

client/allocrunner/taskrunner/task_runner.go

@@ -535,7 +535,8 @@ func (tr *TaskRunner) Run() {
 		return
 	}
 
-	timer, stop := helper.NewSafeTimer(0) // timer duration calculated JIT
+	// start with a stopped timer; actual restart delay computed later
+	timer, stop := helper.NewStoppedTimer()
 	defer stop()
 
 MAIN:

helper/funcs.go

@@ -3,6 +3,7 @@ package helper
 import (
 	"crypto/sha512"
 	"fmt"
+	"math"
 	"net/http"
 	"path/filepath"
 	"reflect"
@@ -368,6 +369,9 @@ type StopFunc func()
 //
 // Returns the time.Timer and also a StopFunc, forcing the caller to deal
 // with stopping the time.Timer to avoid leaking a goroutine.
+//
+// Note: If creating a Timer that should do nothing until Reset is called, use
+// NewStoppedTimer instead for safely creating the timer in a stopped state.
 func NewSafeTimer(duration time.Duration) (*time.Timer, StopFunc) {
 	if duration <= 0 {
 		// Avoid panic by using the smallest positive value. This is close enough
@@ -385,6 +389,25 @@ func NewSafeTimer(duration time.Duration) (*time.Timer, StopFunc) {
 	return t, cancel
 }
 
+// NewStoppedTimer creates a time.Timer in a stopped state. This is useful when
+// the actual wait time will computed and set later via Reset.
+func NewStoppedTimer() (*time.Timer, StopFunc) {
+	t, f := NewSafeTimer(math.MaxInt64)
+	t.Stop()
+	return t, f
+}
+
+// ConvertSlice takes the input slice and generates a new one using the
+// supplied conversion function to covert the element. This is useful when
+// converting a slice of strings to a slice of structs which wraps the string.
+func ConvertSlice[A, B any](original []A, conversion func(a A) B) []B {
+	result := make([]B, len(original))
+	for i, element := range original {
+		result[i] = conversion(element)
+	}
+	return result
+}
+
 // IsMethodHTTP returns whether s is a known HTTP method, ignoring case.
 func IsMethodHTTP(s string) bool {
 	switch strings.ToUpper(s) {

helper/funcs_test.go

@@ -376,7 +376,7 @@ func TestCheckNamespaceScope(t *testing.T) {
 	}
 }
 
-func Test_NewSafeTimer(t *testing.T) {
+func TestTimer_NewSafeTimer(t *testing.T) {
 	t.Run("zero", func(t *testing.T) {
 		timer, stop := NewSafeTimer(0)
 		defer stop()
@@ -390,6 +390,42 @@ func Test_NewSafeTimer(t *testing.T) {
 	})
 }
 
+func TestTimer_NewStoppedTimer(t *testing.T) {
+	timer, stop := NewStoppedTimer()
+	defer stop()
+
+	select {
+	case <-timer.C:
+		must.Unreachable(t)
+	default:
+	}
+}
+
+func Test_ConvertSlice(t *testing.T) {
+	t.Run("string wrapper", func(t *testing.T) {
+
+		type wrapper struct{ id string }
+		input := []string{"foo", "bar", "bad", "had"}
+		cFn := func(id string) *wrapper { return &wrapper{id: id} }
+
+		expectedOutput := []*wrapper{{id: "foo"}, {id: "bar"}, {id: "bad"}, {id: "had"}}
+		actualOutput := ConvertSlice(input, cFn)
+		require.ElementsMatch(t, expectedOutput, actualOutput)
+	})
+
+	t.Run("int wrapper", func(t *testing.T) {
+
+		type wrapper struct{ id int }
+		input := []int{10, 13, 1987, 2020}
+		cFn := func(id int) *wrapper { return &wrapper{id: id} }
+
+		expectedOutput := []*wrapper{{id: 10}, {id: 13}, {id: 1987}, {id: 2020}}
+		actualOutput := ConvertSlice(input, cFn)
+		require.ElementsMatch(t, expectedOutput, actualOutput)
+
+	})
+}
+
 func Test_IsMethodHTTP(t *testing.T) {
 	t.Run("is method", func(t *testing.T) {
 		cases := []string{