Fix goroutine leak

pkg/runner/test_runner.go

@@ -1,3 +1,8 @@
+// Copyright (c) Facebook, Inc. and its affiliates.
+//
+// This source code is licensed under the MIT license found in the
+// LICENSE file in the root directory of this source tree.
+
 package runner
 
 import (
@@ -333,16 +338,19 @@ func (tr *testRunner) waitStepRunners(ctx statectx.Context) error {
 	case <-time.After(tr.shutdownTimeout):
 		tr.log.Errorf("step runners failed to shut down correctly")
 		// If there is a step with an error set, use that.
-		if err := tr.checkStepRunners(); err != nil {
-			return err
-		}
+		err := tr.checkStepRunners()
 		// If there isn't, enumerate ones that were still running at the time.
-		err := &cerrors.ErrTestStepsNeverReturned{}
+		nrerr := &cerrors.ErrTestStepsNeverReturned{}
+		if err == nil {
+			err = nrerr
+		}
 		tr.mu.Lock()
 		defer tr.mu.Unlock()
 		for _, ss := range tr.steps {
 			if ss.stepRunning {
-				err.StepNames = append(err.StepNames, ss.sb.TestStepLabel)
+				nrerr.StepNames = append(nrerr.StepNames, ss.sb.TestStepLabel)
+				// We cannot make the step itself return but we can at least release the reader.
+				tr.safeCloseOutCh(ss)
 			}
 		}
 		return err

tests/plugins/teststeps/badtargets/badtargets.go

@@ -41,15 +41,31 @@ func (ts *badTargets) Run(ctx statectx.Context, ch test.TestStepChannels, params
 			case "TGood":
 				// We should not depend on pointer matching, so emit a copy.
 				target2 := *target
-				ch.Out <- &target2
+				select {
+				case ch.Out <- &target2:
+				case <-ctx.Done():
+					return statectx.ErrCanceled
+				}
 			case "TDup":
-				ch.Out <- target
-				ch.Out <- target
+				select {
+				case ch.Out <- target:
+				case <-ctx.Done():
+					return statectx.ErrCanceled
+				}
+				select {
+				case ch.Out <- target:
+				case <-ctx.Done():
+					return statectx.ErrCanceled
+				}
 			default:
 				// Mangle the returned target name.
 				target2 := *target
 				target2.ID = target2.ID + "XXX"
-				ch.Out <- &target2
+				select {
+				case ch.Out <- &target2:
+				case <-ctx.Done():
+					return statectx.ErrCanceled
+				}
 			}
 		case <-ctx.Done():
 			return nil