From 8f00a7bf5af0cb47d6620da7f7d61f4c5d780588 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Mon, 25 Sep 2023 13:50:55 -0700
Subject: [PATCH] stackruntime: Make sure PlanResponse and ApplyResponse
 channels get closed

The closure of these channels is how a caller knows that the operation has
completed, so we must always close them before we return or the caller
is likely to get wedged.
---
 internal/stacks/stackruntime/apply.go |  9 +++++++--
 internal/stacks/stackruntime/plan.go  | 11 +++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/internal/stacks/stackruntime/apply.go b/internal/stacks/stackruntime/apply.go
index 1e92d6def8..1585cf2b78 100644
--- a/internal/stacks/stackruntime/apply.go
+++ b/internal/stacks/stackruntime/apply.go
@@ -44,6 +44,13 @@ func Apply(ctx context.Context, req *ApplyRequest, resp *ApplyResponse) {
 		},
 	}
 
+	// Whatever return path we take, we must close our channels to allow
+	// a caller to see that the operation is complete.
+	defer func() {
+		close(resp.Diagnostics)
+		close(resp.AppliedChanges) // MUST be the last channel to close
+	}()
+
 	main, err := stackeval.ApplyPlan(
 		ctx,
 		req.Config,
@@ -78,8 +85,6 @@ func Apply(ctx context.Context, req *ApplyRequest, resp *ApplyResponse) {
 		resp.Diagnostics <- diag
 	}
 
-	close(resp.Diagnostics)
-	close(resp.AppliedChanges) // MUST be the last channel to close
 }
 
 // ApplyRequest represents the inputs to an [Apply] call.
diff --git a/internal/stacks/stackruntime/plan.go b/internal/stacks/stackruntime/plan.go
index b630ed5d88..be2c855fff 100644
--- a/internal/stacks/stackruntime/plan.go
+++ b/internal/stacks/stackruntime/plan.go
@@ -26,9 +26,15 @@ import (
 // through resp after passing it to this function, aside from the implicit
 // modifications to the internal state of channels caused by reading them.
 func Plan(ctx context.Context, req *PlanRequest, resp *PlanResponse) {
+	var respMu sync.Mutex // must hold this when accessing fields of resp, aside from channel sends
 	resp.Applyable = true // we'll reset this to false later if appropriate
 
-	var respMu sync.Mutex // must hold this when accessing fields of resp, aside from channel sends
+	// Whatever return path we take, we must close our channels to allow
+	// a caller to see that the operation is complete.
+	defer func() {
+		close(resp.Diagnostics)
+		close(resp.PlannedChanges) // MUST be the last channel to close
+	}()
 
 	main := stackeval.NewForPlanning(req.Config, stackeval.PlanOpts{
 		InputVariableValues: req.InputValues,
@@ -66,9 +72,6 @@ func Plan(ctx context.Context, req *PlanRequest, resp *PlanResponse) {
 	resp.PlannedChanges <- &stackplan.PlannedChangeApplyable{
 		Applyable: resp.Applyable,
 	}
-
-	close(resp.Diagnostics)
-	close(resp.PlannedChanges) // MUST be the last channel to close
 }
 
 // PlanRequest represents the inputs to a [Plan] call.