From 6b93e9c37f1032fb4f5d630221148cc0121371d2 Mon Sep 17 00:00:00 2001
From: Josh Humphries <2035234+jhump@users.noreply.github.com>
Date: Mon, 8 Apr 2024 18:04:38 -0400
Subject: [PATCH 1/2] fix subtle potential race/panic

---
 parser/parser_test.go | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/parser/parser_test.go b/parser/parser_test.go
index e40c37d5..f1736143 100644
--- a/parser/parser_test.go
+++ b/parser/parser_test.go
@@ -22,6 +22,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -1007,11 +1008,23 @@ func TestPathological(t *testing.T) {
 				allowedDuration = 10 * time.Second
 				t.Logf("allowing %v since race detector is enabled", allowedDuration)
 			}
+			var timerDone sync.WaitGroup
+			timerDone.Add(1)
 			timer := time.AfterFunc(allowedDuration, func() {
+				defer timerDone.Done()
 				t.Errorf("test took too long to execute (> %v)", allowedDuration)
 				cancel()
 			})
-			defer timer.Stop()
+			defer func() {
+				if !timer.Stop() {
+					// We must wait for the stop function to finish. Otherwise, we could
+					// tickle a weird panic -- if this function returns and THEN the
+					// timer goroutine tries to report an error using its *testing.T,
+					// the test framework panics:
+					//   Fail in goroutine after TestPathological has completed
+					timerDone.Wait()
+				}
+			}()
 
 			for i := 0; i < 3; i++ {
 				if ctx.Err() != nil {

From 6c3ee5d78bea1c43a3ed9e96b08af46cac6be044 Mon Sep 17 00:00:00 2001
From: Josh Humphries <2035234+jhump@users.noreply.github.com>
Date: Wed, 10 Apr 2024 14:20:32 -0400
Subject: [PATCH 2/2] simplify based on suggestion

---
 parser/parser_test.go | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/parser/parser_test.go b/parser/parser_test.go
index f1736143..0d3282af 100644
--- a/parser/parser_test.go
+++ b/parser/parser_test.go
@@ -22,7 +22,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -994,7 +993,6 @@ func TestPathological(t *testing.T) {
 		fileName, canParse := fileName, testCases[fileName] // don't want test func below to capture loop var
 		t.Run(fileName, func(t *testing.T) {
 			t.Parallel()
-			ctx, cancel := context.WithCancel(context.Background())
 			// Fuzz testing complains if this loop, with 100 iterations, takes longer
 			// than 60 seconds. To prevent this test from being too slow, we limit to
 			// 3 iterations and no longer than 1 second (which is a stricter deadline).
@@ -1008,24 +1006,13 @@ func TestPathological(t *testing.T) {
 				allowedDuration = 10 * time.Second
 				t.Logf("allowing %v since race detector is enabled", allowedDuration)
 			}
-			var timerDone sync.WaitGroup
-			timerDone.Add(1)
-			timer := time.AfterFunc(allowedDuration, func() {
-				defer timerDone.Done()
-				t.Errorf("test took too long to execute (> %v)", allowedDuration)
-				cancel()
-			})
+			ctx, cancel := context.WithTimeout(context.Background(), allowedDuration)
 			defer func() {
-				if !timer.Stop() {
-					// We must wait for the stop function to finish. Otherwise, we could
-					// tickle a weird panic -- if this function returns and THEN the
-					// timer goroutine tries to report an error using its *testing.T,
-					// the test framework panics:
-					//   Fail in goroutine after TestPathological has completed
-					timerDone.Wait()
+				if ctx.Err() != nil {
+					t.Errorf("test took too long to execute (> %v)", allowedDuration)
 				}
+				cancel()
 			}()
-
 			for i := 0; i < 3; i++ {
 				if ctx.Err() != nil {
 					break