fix: command manager connect go routine leak

internals/overlord/cmdstate/export_test.go

@@ -0,0 +1,24 @@
+// Copyright (c) 2026 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmdstate
+
+// AddTestExecution inserts a fake execution into the manager's map and
+// broadcasts the condition variable.
+func (m *CommandManager) AddTestExecution(taskID string) {
+	m.executionsCond.L.Lock()
+	m.executions[taskID] = &execution{}
+	m.executionsCond.Broadcast()
+	m.executionsCond.L.Unlock()
+}

internals/overlord/cmdstate/manager.go

@@ -68,10 +68,12 @@ func (m *CommandManager) Connect(r *http.Request, w http.ResponseWriter, task *s
 		m.executionsCond.L.Unlock()
 	}()
 
-	executionCh := make(chan *execution)
+	executionCh := make(chan *execution, 1)
 	go func() {
 		e := m.waitExecution(task.ID(), stopWait)
 		if e != nil {
+			// executionCh is buffered to avoid a goroutine leak when this is
+			// blocked indefinitely.
 			executionCh <- e
 		}
 	}()

internals/overlord/cmdstate/manager_test.go

@@ -0,0 +1,60 @@
+// Copyright (c) 2026 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmdstate_test
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+
+	. "gopkg.in/check.v1"
+
+	"github.com/canonical/pebble/internals/overlord/cmdstate"
+	"github.com/canonical/pebble/internals/overlord/state"
+)
+
+type managerSuite struct{}
+
+var _ = Suite(&managerSuite{})
+
+// TestConnectContextCancelledGoroutineLeak demonstrates the goroutine leak in
+// Connect. executionCh is unbuffered: if Connect exits via r.Context().Done()
+// after waitExecution has already found a non-nil execution, the goroutine
+// blocks forever trying to send on executionCh with no receiver.
+//
+// The test is probabilistic.
+func (s *managerSuite) TestConnectContextCancelledGoroutineLeak(c *C) {
+	st := state.New(nil)
+	runner := state.NewTaskRunner(st)
+	mgr := cmdstate.NewManager(runner)
+
+	st.Lock()
+	task := st.NewTask("exec", "test cmd")
+	chg := st.NewChange("exec", "test change")
+	chg.AddTask(task)
+	st.Unlock()
+
+	// Pre-register the execution so waitExecution returns immediately
+	// with a non-nil value, before stopWait can be closed.
+	mgr.AddTestExecution(task.ID())
+
+	// Pre-cancel the context so Connect can exit via r.Context().Done()
+	// while the goroutine is still trying to send on executionCh.
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil).WithContext(ctx)
+	_ = mgr.Connect(r, httptest.NewRecorder(), task, "stdio")
+}

internals/overlord/cmdstate/package_test.go

@@ -0,0 +1,26 @@
+// Copyright (c) 2026 Canonical Ltd
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License version 3 as
+// published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmdstate_test
+
+import (
+	"testing"
+
+	"github.com/canonical/pebble/internals/testutil"
+	. "gopkg.in/check.v1"
+)
+
+func Test(t *testing.T) {
+	testutil.PrintGoroutineLeaks(t, TestingT)
+}