From 2d0b3a300ff2b9a4951a1cd7ce8345629d399487 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le <cuong@orijtech.com>
Date: Tue, 25 May 2021 16:36:11 +0700
Subject: [PATCH] libs/clist: fix flaky tests (#6453)

To make sure finalizers run, we use channel for synchronization, and a
separate goroutine for trigger runtime.GC every 1 second. In practice,
just two consecutive runtime.GC calls can make all finalizers will run,
but using a separate goroutine make the code more robust and not depend
on garbage collector internal implementation.

Fixes #6452
---
 libs/clist/clist_test.go | 81 ++++++++++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 28 deletions(-)

diff --git a/libs/clist/clist_test.go b/libs/clist/clist_test.go
index aa5142a38..4b222e3d5 100644
--- a/libs/clist/clist_test.go
+++ b/libs/clist/clist_test.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	mrand "math/rand"
 	"runtime"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -65,17 +64,11 @@ func TestSmall(t *testing.T) {
 
 }
 
-// This test is quite hacky because it relies on SetFinalizer
-// which isn't guaranteed to run at all.
-//nolint:unused,deadcode
-func _TestGCFifo(t *testing.T) {
-	if runtime.GOARCH != "amd64" {
-		t.Skipf("Skipping on non-amd64 machine")
-	}
+func TestGCFifo(t *testing.T) {
 
 	const numElements = 1000000
 	l := New()
-	gcCount := new(uint64)
+	gcCount := 0
 
 	// SetFinalizer doesn't work well with circular structures,
 	// so we construct a trivial non-circular structure to
@@ -83,14 +76,14 @@ func _TestGCFifo(t *testing.T) {
 	type value struct {
 		Int int
 	}
-	done := make(chan struct{})
 
+	gcCh := make(chan struct{})
 	for i := 0; i < numElements; i++ {
 		v := new(value)
 		v.Int = i
 		l.PushBack(v)
 		runtime.SetFinalizer(v, func(v *value) {
-			atomic.AddUint64(gcCount, 1)
+			gcCh <- struct{}{}
 		})
 	}
 
@@ -102,25 +95,36 @@ func _TestGCFifo(t *testing.T) {
 		// oldEl.DetachNext()
 	}
 
-	runtime.GC()
-	time.Sleep(time.Second * 3)
-	runtime.GC()
-	time.Sleep(time.Second * 3)
-	_ = done
+	tickerQuitCh := make(chan struct{})
+	tickerDoneCh := make(chan struct{})
+	go func() {
+		defer close(tickerDoneCh)
+		ticker := time.NewTicker(time.Second)
+		for {
+			select {
+			case <-ticker.C:
+				runtime.GC()
+			case <-tickerQuitCh:
+				return
+			}
+		}
+	}()
+
+	for i := 0; i < numElements; i++ {
+		<-gcCh
+		gcCount++
+	}
+
+	close(tickerQuitCh)
+	<-tickerDoneCh
 
-	if *gcCount != numElements {
+	if gcCount != numElements {
 		t.Errorf("expected gcCount to be %v, got %v", numElements,
-			*gcCount)
+			gcCount)
 	}
 }
 
-// This test is quite hacky because it relies on SetFinalizer
-// which isn't guaranteed to run at all.
-//nolint:unused,deadcode
-func _TestGCRandom(t *testing.T) {
-	if runtime.GOARCH != "amd64" {
-		t.Skipf("Skipping on non-amd64 machine")
-	}
+func TestGCRandom(t *testing.T) {
 
 	const numElements = 1000000
 	l := New()
@@ -133,12 +137,13 @@ func _TestGCRandom(t *testing.T) {
 		Int int
 	}
 
+	gcCh := make(chan struct{})
 	for i := 0; i < numElements; i++ {
 		v := new(value)
 		v.Int = i
 		l.PushBack(v)
 		runtime.SetFinalizer(v, func(v *value) {
-			gcCount++
+			gcCh <- struct{}{}
 		})
 	}
 
@@ -153,8 +158,28 @@ func _TestGCRandom(t *testing.T) {
 		_ = el.Next()
 	}
 
-	runtime.GC()
-	time.Sleep(time.Second * 3)
+	tickerQuitCh := make(chan struct{})
+	tickerDoneCh := make(chan struct{})
+	go func() {
+		defer close(tickerDoneCh)
+		ticker := time.NewTicker(time.Second)
+		for {
+			select {
+			case <-ticker.C:
+				runtime.GC()
+			case <-tickerQuitCh:
+				return
+			}
+		}
+	}()
+
+	for i := 0; i < numElements; i++ {
+		<-gcCh
+		gcCount++
+	}
+
+	close(tickerQuitCh)
+	<-tickerDoneCh
 
 	if gcCount != numElements {
 		t.Errorf("expected gcCount to be %v, got %v", numElements,