]> Cypherpunks repositories - gostls13.git/commit
time: more flake removal in asynctimerchan test
authorRuss Cox <rsc@golang.org>
Tue, 14 May 2024 17:21:22 +0000 (13:21 -0400)
committerGopher Robot <gobot@golang.org>
Tue, 14 May 2024 18:27:21 +0000 (18:27 +0000)
commit2cc42f73287e3ad7a11d7296762b2b9fed3a2447
tree45959e6b07871dd5d0db3b0b8cffc33f7758d103
parent0767ffdf27cc3fad4e88eee523fac04f2e669e82
time: more flake removal in asynctimerchan test

Trying to write a test for the corner cases in the old async timer chan
implementation may have been a mistake, especially since this isn't
going to be the default timer chan implementation anymore.
But let's try one more time to fix the test.

I reproduced the remaining builder failures on my Mac laptop
by overloading the CPU in one window and then running 48 instances
of the flaky test in loops using 'stress' in another window.

It turns out that, contrary to my understanding of async timers
and therefore contrary to what the test expected, it is technically
possible for

t := time.NewTicker(1)
t.Reset(1000*time.Hour)
<-t.C
<-t.C

to observe two time values on t.C, as opposed to blocking forever.

We always expect the first time value, since the ticker goes off
immediately (after 1ns) and sends that value into the channel buffer.
To get the second value, the ticker has to be in the process of
going off (which it is doing constantly anyway), and the timer
goroutine has to be about to call sendTime and then get rescheduled.
Then t.Reset and the first <-t.C have to happen.
Then the timer goroutine gets rescheduled and can run sendTime's
non-blocking send on t.C, which finds an empty buffer and writes
a value.

This is unlikely, of course, but it definitely happens. This program
always panics in just a second or two on my laptop:

package main

import (
"os"
"time"
)

func main() {
os.Setenv("GODEBUG", "asynctimerchan=1")
for  {
go func() {
t := time.NewTicker(1)
t.Reset(1000*time.Hour)
<-t.C
select {
case <-t.C:
panic("two receives")
case <-time.After(1*time.Second):
}
}()
}
}

Because I did not understand this nuance, the test did not expect it.

This CL rewrites the test to expect that possibility. I can no longer
make the test fail under 'stress' on my laptop.

For #66322.

Change-Id: I15c75d2c6f24197c43094da20d6ab55306a0a9f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/585359
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
src/time/tick_test.go