Skip to content

Commit b877f04

Browse files
mknyszekgopherbot
authored andcommitted
crypto/tls: add scheduler call to TestCertCache refcount timeout loop
Currently TestCertCache will busy loop waiting for a cleanup (in the runtime.AddCleanup sense) to execute. If we ever get into this busy loop, then on single-threaded platforms like js/wasm, we'll end up _always_ timing out. This doesn't happen right now because we're getting lucky. The finalizer goroutine is scheduled into the runnext slot with 'ready' and is thus scheduled immediately after the GC call. In a follow-up CL, scheduling cleanup goroutines becomes less aggressive, and thus this test fails. Although perhaps that CL should schedule cleanup goroutines more aggressively, the test is still technically buggy, because it expects busy loops like this to call into the scheduler, but that won't happen on certain platforms. Change-Id: I8efe5975be97f4314aec1c8c6e9e22f396be9c94 Reviewed-on: https://go-review.googlesource.com/c/go/+/670755 Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Roland Shoemaker <[email protected]> Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent c9d0fad commit b877f04

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

src/crypto/tls/cache_test.go

+13-3
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,22 @@ func TestCertCache(t *testing.T) {
4141

4242
timeoutRefCheck := func(t *testing.T, key string, count int64) {
4343
t.Helper()
44-
c := time.After(4 * time.Second)
44+
45+
// Explicitly check every 1 ms up to the timeout instead of busy-looping.
46+
//
47+
// On single-threaded platforms like js/wasm a busy-loop might
48+
// never call into the scheduler for the full timeout, meaning
49+
// that if we arrive here and the cleanup hasn't already run,
50+
// we'll simply loop until the timeout. Busy-loops put us at the
51+
// mercy of the Go scheduler, making this test fragile on some
52+
// platforms.
53+
timeout := time.After(4 * time.Second)
54+
check := time.After(1 * time.Millisecond)
4555
for {
4656
select {
47-
case <-c:
57+
case <-timeout:
4858
t.Fatal("timed out waiting for expected ref count")
49-
default:
59+
case <-check:
5060
e, ok := cc.Load(key)
5161
if !ok && count != 0 {
5262
t.Fatal("cache does not contain expected key")

0 commit comments

Comments
 (0)