From 299e951d8629423689c679fe9da64e38b25ae58e Mon Sep 17 00:00:00 2001 From: joshjennings98 Date: Mon, 18 Aug 2025 16:07:56 +0100 Subject: [PATCH] :bug: `subprocess` Make sure that cancellation can't enter deadlock --- changes/20250818160732.bugfix | 1 + utils/subprocess/monitoring.go | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changes/20250818160732.bugfix diff --git a/changes/20250818160732.bugfix b/changes/20250818160732.bugfix new file mode 100644 index 0000000000..19bd41c0ca --- /dev/null +++ b/changes/20250818160732.bugfix @@ -0,0 +1 @@ +:bug: `subprocess` Make sure that cancellation can't enter deadlock diff --git a/utils/subprocess/monitoring.go b/utils/subprocess/monitoring.go index 8e9ebbe7e3..e8ea8584b2 100644 --- a/utils/subprocess/monitoring.go +++ b/utils/subprocess/monitoring.go @@ -34,7 +34,16 @@ func newSubprocessMonitoring(parentCtx context.Context) *subprocessMonitoring { // CancelSubprocess interrupts an on-going process. func (s *subprocessMonitoring) CancelSubprocess() { - s.monitoringStopping.Store(true) + // Ensure we only ever run the cancel-store once and prevent the following deadlocks: + // 1. Some functions like Execute() do defer s.Cancel() + // 2. This calls subprocessMonitoring.CancelSubprocess() which calls cancelStore.Cancel() (acquiring mutex) + // 3. That Cancel() calls the context cancel func, which closes ProcessContext().Done() + // 4. The runProcessMonitoring blocks on ctx<-Done() and calls m.CancelSubprocess() again + // 5. This tries to run cancelStore.Cancel() a second time while the first Cancel() is still executing + // 6. go-deadlock detects deadlock + if s.monitoringStopping.Swap(true) { + return + } s.cancelStore.Cancel() }