mirror of
https://github.com/k8sgpt-ai/k8sgpt.git
synced 2026-07-17 02:04:40 +00:00
fix: clamp max-concurrency in custom analysis to avoid deadlock and panic (#1700)
RunCustomAnalysis built its worker semaphore directly from the unvalidated
a.MaxConcurrency field, unlike RunAnalysis which clamps the value first. With
the user-supplied --max-concurrency flag this caused two failures:
- --max-concurrency 0 makes an unbuffered channel, so the pre-send blocks
waiting for a receiver that is only launched on the next line, deadlocking
the whole analyze command whenever a custom analyzer is configured.
- --max-concurrency -1 makes make(chan struct{}, -1) panic with
"makechan: size out of range".
Apply the same lower/upper bound clamp RunAnalysis already uses (default 10 for
non-positive values, cap at 100). Add regression tests covering both cases.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
Co-authored-by: Alex Jones <1235925+AlexsJones@users.noreply.github.com>
This commit is contained in:
@@ -260,7 +260,16 @@ func (a *Analysis) RunCustomAnalysis() {
|
||||
return
|
||||
}
|
||||
|
||||
semaphore := make(chan struct{}, a.MaxConcurrency)
|
||||
// Set a reasonable maximum for concurrency to prevent excessive memory allocation
|
||||
const maxAllowedConcurrency = 100
|
||||
concurrency := a.MaxConcurrency
|
||||
if concurrency <= 0 {
|
||||
concurrency = 10 // Default value if not set
|
||||
} else if concurrency > maxAllowedConcurrency {
|
||||
concurrency = maxAllowedConcurrency // Cap at a reasonable maximum
|
||||
}
|
||||
|
||||
semaphore := make(chan struct{}, concurrency)
|
||||
var wg sync.WaitGroup
|
||||
var mutex sync.Mutex
|
||||
verbose := viper.GetBool("verbose")
|
||||
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/agiledragon/gomonkey/v2"
|
||||
"github.com/k8sgpt-ai/k8sgpt/pkg/ai"
|
||||
@@ -634,6 +635,48 @@ func TestVerbose_RunCustomAnalysisWithCustomAnalyzer(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Test: RunCustomAnalysis must not deadlock when MaxConcurrency is 0.
|
||||
// A non-positive value previously produced an unbuffered semaphore whose only
|
||||
// receiver is launched after the blocking send, hanging the command forever.
|
||||
func TestRunCustomAnalysisZeroConcurrency(t *testing.T) {
|
||||
viper.Set("custom_analyzers", []map[string]interface{}{
|
||||
{
|
||||
"name": "TestCustomAnalyzer",
|
||||
"connection": map[string]interface{}{"url": "127.0.0.1", "port": "2333"},
|
||||
},
|
||||
})
|
||||
|
||||
analysisObj := &Analysis{
|
||||
MaxConcurrency: 0,
|
||||
}
|
||||
|
||||
done := make(chan struct{})
|
||||
go func() {
|
||||
analysisObj.RunCustomAnalysis()
|
||||
close(done)
|
||||
}()
|
||||
|
||||
select {
|
||||
case <-done:
|
||||
case <-time.After(10 * time.Second):
|
||||
t.Fatal("RunCustomAnalysis deadlocked with MaxConcurrency=0")
|
||||
}
|
||||
}
|
||||
|
||||
// Test: RunCustomAnalysis must not panic when MaxConcurrency is negative.
|
||||
// make(chan struct{}, negative) previously panicked with "makechan: size out of range".
|
||||
func TestRunCustomAnalysisNegativeConcurrency(t *testing.T) {
|
||||
viper.Set("custom_analyzers", []interface{}{})
|
||||
|
||||
analysisObj := &Analysis{
|
||||
MaxConcurrency: -1,
|
||||
}
|
||||
|
||||
require.NotPanics(t, func() {
|
||||
analysisObj.RunCustomAnalysis()
|
||||
})
|
||||
}
|
||||
|
||||
// Test: Verbose output in GetAIResults
|
||||
func TestVerbose_GetAIResults(t *testing.T) {
|
||||
viper.Set("verbose", true)
|
||||
|
||||
Reference in New Issue
Block a user