Skip to content

Commit d92e058

Browse files
tonyredondotonyredondo
committed
fix(ci-visibility): release parallel subtest scheduler slot
1 parent 1645ac3 commit d92e058

3 files changed

Lines changed: 93 additions & 25 deletions

File tree

‎internal/civisibility/integrations/gotesting/failnow/failnow_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ func TestCleanupRunsAfterParallelSubtest(t *testing.T) {
8080
runTestScenario(t, "test-cleanup-after-parallel-subtest", "^TestCleanupRunsAfterParallelSubtestFixture$")
8181
}
8282

83+
// TestParallelSubtestSchedulerSlotIsReleased verifies Datadog-managed retry
84+
// clones release their parent scheduler slot before waiting for parallel
85+
// subtests. With -parallel=1, failing to release that slot deadlocks the child
86+
// in testing.(*testState).waitParallel until the package timeout fires.
87+
func TestParallelSubtestSchedulerSlotIsReleased(t *testing.T) {
88+
runSubprocess(t, "test-cleanup-after-parallel-subtest", "-test.run", "^TestCleanupRunsAfterParallelSubtestFixture$", "-test.parallel=1", "-test.timeout=5s")
89+
}
90+
8391
func TestFlakyRetryGlobalBudget(t *testing.T) {
8492
runTestScenario(t, "test-flaky-retry-global-budget", "^TestFlakyRetryGlobalBudgetFixture$")
8593
}

‎internal/civisibility/integrations/gotesting/instrumentation.go‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ func executeTestIteration(execOpts *executionOptions) bool {
833833
*cn <- struct{}{}
834834
}()
835835
defer func() {
836-
completeParallelSubtests(localTPrivateFields)
836+
completeParallelSubtests(pLocalT, localTPrivateFields)
837837
}()
838838
defer func() {
839839
duration = time.Since(startTime)
@@ -912,7 +912,7 @@ func executeTestIteration(execOpts *executionOptions) bool {
912912
// cleanup Goexit in a helper goroutine so retry orchestration can treat cleanup
913913
// failures as attempt failures instead of letting them escape the retry loop.
914914
func runTestCleanup(t *testing.T, result *testCleanupResult) {
915-
completeParallelSubtests(getTestPrivateFields(t))
915+
completeParallelSubtests(t, getTestPrivateFields(t))
916916
result.ran = true
917917
done := make(chan struct{})
918918
go func() {
@@ -933,15 +933,20 @@ func runTestCleanup(t *testing.T, result *testCleanupResult) {
933933
}
934934

935935
// completeParallelSubtests releases and waits for parallel subtests owned by a
936-
// Datadog-managed clone. It clears the subtest queue before releasing the
937-
// barrier so later cleanup paths cannot close the same barrier twice.
938-
func completeParallelSubtests(localTPrivateFields *commonPrivateFields) {
936+
// Datadog-managed clone. It mirrors testing.tRunner's scheduler accounting:
937+
// release the parent slot before unblocking children, then reacquire it for
938+
// sequential parents before running cleanup.
939+
func completeParallelSubtests(t *testing.T, localTPrivateFields *commonPrivateFields) {
939940
if localTPrivateFields == nil || localTPrivateFields.sub == nil || len(*localTPrivateFields.sub) == 0 {
940941
return
941942
}
942943

943944
subtests := *localTPrivateFields.sub
944945
*localTPrivateFields.sub = nil
946+
testState := getTestState(t)
947+
if testState != nil {
948+
testingTestStateRelease(testState)
949+
}
945950
if localTPrivateFields.barrier != nil && *localTPrivateFields.barrier != nil {
946951
close(*localTPrivateFields.barrier)
947952
}
@@ -951,6 +956,24 @@ func completeParallelSubtests(localTPrivateFields *commonPrivateFields) {
951956
<-*pvSub.signal
952957
}
953958
}
959+
if testState != nil && !isParallelTest(t, localTPrivateFields) {
960+
testingTestStateWaitParallel(testState)
961+
}
962+
}
963+
964+
// isParallelTest reports whether the active test has entered Go's parallel-test
965+
// path. Datadog-managed retry clones forward Parallel to the original *testing.T,
966+
// so the original must also be checked before deciding whether to reacquire the
967+
// scheduler slot.
968+
func isParallelTest(t *testing.T, localTPrivateFields *commonPrivateFields) bool {
969+
if localTPrivateFields != nil && localTPrivateFields.isParallel != nil && *localTPrivateFields.isParallel {
970+
return true
971+
}
972+
if execMeta := getTestMetadata(t); execMeta != nil && execMeta.originalTest != nil {
973+
originalFields := getTestPrivateFields(execMeta.originalTest)
974+
return originalFields != nil && originalFields.isParallel != nil && *originalFields.isParallel
975+
}
976+
return false
954977
}
955978

956979
// runAndApplyTestCleanup runs a retry attempt's cleanups before its span is
@@ -1019,3 +1042,9 @@ func (m *noopMutex) TryLock() bool { return true }
10191042

10201043
//go:linkname testingTRunCleanup testing.(*common).runCleanup
10211044
func testingTRunCleanup(c *testing.T, ph int) (panicVal any)
1045+
1046+
//go:linkname testingTestStateWaitParallel testing.(*testState).waitParallel
1047+
func testingTestStateWaitParallel(s *testingTestState)
1048+
1049+
//go:linkname testingTestStateRelease testing.(*testState).release
1050+
func testingTestStateRelease(s *testingTestState)

‎internal/civisibility/integrations/gotesting/reflections.go‎

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -78,16 +78,17 @@ func copyFieldUsingPointersWithConversion[V any](source any, target any, fieldNa
7878

7979
// commonPrivateFields is collection of required private fields from testing.common
8080
type commonPrivateFields struct {
81-
mu *sync.RWMutex
82-
output *[]byte // Output generated by test or benchmark.
83-
level *int
84-
name *string // Name of test or benchmark.
85-
failed *bool // Test or benchmark has failed.
86-
skipped *bool // Test or benchmark has been skipped.
87-
parent *unsafe.Pointer // Parent common
88-
barrier *chan bool // Barrier for parallel tests
89-
signal *chan bool // Signal channel for test completion
90-
sub *[]*testing.T // Queue of subtests to be run in parallel.
81+
mu *sync.RWMutex
82+
output *[]byte // Output generated by test or benchmark.
83+
level *int
84+
name *string // Name of test or benchmark.
85+
failed *bool // Test or benchmark has failed.
86+
skipped *bool // Test or benchmark has been skipped.
87+
isParallel *bool // Whether the test has called testing.T.Parallel.
88+
parent *unsafe.Pointer // Parent common.
89+
barrier *chan bool // Barrier for parallel tests.
90+
signal *chan bool // Signal channel for test completion.
91+
sub *[]*testing.T // Queue of subtests to be run in parallel.
9192
}
9293

9394
// AddLevel increase or decrease the testing.common.level field value, used by
@@ -188,16 +189,17 @@ func getTestPrivateFieldsFast(t *testing.T, layout *testingInternalsLayout) *com
188189
return nil
189190
}
190191
fields := &commonPrivateFields{
191-
mu: fieldPtr[sync.RWMutex](commonBase, layout.common.mu),
192-
output: fieldPtr[[]byte](commonBase, layout.common.output),
193-
level: fieldPtr[int](commonBase, layout.common.level),
194-
name: fieldPtr[string](commonBase, layout.common.name),
195-
failed: fieldPtr[bool](commonBase, layout.common.failed),
196-
skipped: fieldPtr[bool](commonBase, layout.common.skipped),
197-
parent: (*unsafe.Pointer)(fieldRawPtr(commonBase, layout.common.parent.unsafeField)),
198-
barrier: fieldPtr[chan bool](commonBase, layout.common.barrier),
199-
signal: fieldPtr[chan bool](commonBase, layout.common.signal),
200-
sub: fieldPtr[[]*testing.T](commonBase, layout.common.sub),
192+
mu: fieldPtr[sync.RWMutex](commonBase, layout.common.mu),
193+
output: fieldPtr[[]byte](commonBase, layout.common.output),
194+
level: fieldPtr[int](commonBase, layout.common.level),
195+
name: fieldPtr[string](commonBase, layout.common.name),
196+
failed: fieldPtr[bool](commonBase, layout.common.failed),
197+
skipped: fieldPtr[bool](commonBase, layout.common.skipped),
198+
isParallel: fieldPtr[bool](commonBase, layout.common.isParallel),
199+
parent: (*unsafe.Pointer)(fieldRawPtr(commonBase, layout.common.parent.unsafeField)),
200+
barrier: fieldPtr[chan bool](commonBase, layout.common.barrier),
201+
signal: fieldPtr[chan bool](commonBase, layout.common.signal),
202+
sub: fieldPtr[[]*testing.T](commonBase, layout.common.sub),
201203
}
202204
runtime.KeepAlive(t)
203205
return fields
@@ -227,6 +229,9 @@ func getTestPrivateFieldsReflect(t *testing.T) *commonPrivateFields {
227229
if ptr, err := getFieldPointerFrom(t, "skipped"); err == nil && ptr != nil {
228230
testFields.skipped = (*bool)(ptr)
229231
}
232+
if ptr, err := getFieldPointerFrom(t, "isParallel"); err == nil && ptr != nil {
233+
testFields.isParallel = (*bool)(ptr)
234+
}
230235
if ptr, err := getFieldPointerFrom(t, "parent"); err == nil && ptr != nil {
231236
testFields.parent = (*unsafe.Pointer)(ptr)
232237
}
@@ -243,6 +248,32 @@ func getTestPrivateFieldsReflect(t *testing.T) *commonPrivateFields {
243248
return testFields
244249
}
245250

251+
// testingTestState is an opaque handle for testing.testState. The real type is
252+
// private to the standard library; callers must only pass values obtained from
253+
// a *testing.T's private tstate field.
254+
type testingTestState struct{}
255+
256+
// getTestState returns the private testing.testState pointer used by Go's
257+
// parallel-test scheduler. A nil result means this Go runtime layout is not
258+
// supported by the scheduler-slot helper.
259+
func getTestState(t *testing.T) *testingTestState {
260+
layout := getTestingInternalsLayout()
261+
if layout != nil && !layout.disabled && layout.tstate.available {
262+
ptr := fieldRawPtr(unsafe.Pointer(t), layout.tstate.unsafeField)
263+
if ptr == nil {
264+
return nil
265+
}
266+
state := *(**testingTestState)(ptr)
267+
runtime.KeepAlive(t)
268+
return state
269+
}
270+
271+
if ptr, err := getFieldPointerFrom(t, "tstate"); err == nil && ptr != nil {
272+
return *(**testingTestState)(ptr)
273+
}
274+
return nil
275+
}
276+
246277
// getTestParentPrivateFields is a method to retrieve all required parent privates field from
247278
// testing.T.parent, returning a commonPrivateFields instance
248279
func getTestParentPrivateFields(t *testing.T) *commonPrivateFields {

0 commit comments

Comments
 (0)