From e0daf3c7851150c1b1a63718ab74dfd83c6b491e Mon Sep 17 00:00:00 2001 From: Jeremiah Gowdy Date: Sun, 3 Aug 2025 10:10:51 -0700 Subject: [PATCH 1/2] Add comprehensive memory leak detection tests This implements comprehensive memory leak detection for Asherah's critical components: **Memory Leak Detection Tests:** - Key cache operations (GetOrLoad, GetOrLoadLatest, unique vs same keys) - Cached crypto key reference counting (increment/decrement cycles, edge cases) - Session factory and session lifecycle management - Encrypt/decrypt operations with various payload sizes - Session caching behavior under load - Large payload operations for memory pressure testing **Goroutine Leak Detection:** - Session factory creation/cleanup cycles - Concurrent key cache access patterns - Background goroutine management - Tolerance for expected system goroutines **Key Features:** - Automatic garbage collection before/after measurements - Configurable memory growth tolerance (5MB default) - Comprehensive edge case testing (double close, increment after close) - Race-condition safe (all tests pass with -race) - Validation tests to verify leak detection works (skipped by default) - Fast execution (most tests complete in <1 second) **Test Coverage:** - Reference counting correctness - Cache eviction behavior - Session lifecycle management - Memory growth tracking with GC awareness - Goroutine lifecycle management These tests enable early detection of memory leaks during development and provide confidence in production memory stability. Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- go/appencryption/memory_leak_test.go | 611 ++++++++++++++++++ .../memory_leak_validation_test.go | 208 ++++++ 2 files changed, 819 insertions(+) create mode 100644 go/appencryption/memory_leak_test.go create mode 100644 go/appencryption/memory_leak_validation_test.go diff --git a/go/appencryption/memory_leak_test.go b/go/appencryption/memory_leak_test.go new file mode 100644 index 000000000..ecb034b84 --- /dev/null +++ b/go/appencryption/memory_leak_test.go @@ -0,0 +1,611 @@ +package appencryption + +import ( + "context" + "fmt" + "runtime" + "testing" + "time" + + "github.com/godaddy/asherah/go/appencryption/internal" + "github.com/godaddy/asherah/go/securememory/memguard" + "github.com/stretchr/testify/require" +) + +// Memory leak detection tests for Asherah Go implementation +// These tests help identify memory leaks in key caches, sessions, and crypto key reference counting +// +// Usage: +// go test -run "MemoryLeaks" -v # Run all memory leak tests +// go test -race -run "MemoryLeaks" -v # Run with race detection +// go test -run TestKeyCache_MemoryLeaks -v # Run specific test category +// go test -run TestGoroutineLeaks -v # Run goroutine leak tests +// +// Memory leak tests automatically: +// - Force garbage collection before/after measurements +// - Track memory allocation growth with configurable tolerance (5MB default) +// - Detect goroutine leaks with tolerance for background goroutines +// - Test reference counting edge cases +// - Validate session and key cache lifecycle management +// +// Tests are designed to be: +// - Fast (most complete in <1 second) +// - Reliable (tolerant to GC timing and background activity) +// - Comprehensive (cover all major allocation paths) +// - Race-condition safe (all tests pass with -race) + +const ( + memLeakTestIterations = 1000 + memLeakToleranceMB = 5 // MB tolerance for memory growth +) + +var ( + memLeakSecretFactory = new(memguard.SecretFactory) +) + +// Create minimal test implementations to avoid import cycles + +type benchmarkMetastore struct{} + +func (m *benchmarkMetastore) Load(ctx context.Context, keyID string, created int64) (*EnvelopeKeyRecord, error) { + return nil, nil // Simulate no existing key +} + +func (m *benchmarkMetastore) LoadLatest(ctx context.Context, keyID string) (*EnvelopeKeyRecord, error) { + return nil, nil // Simulate no existing key +} + +func (m *benchmarkMetastore) Store(ctx context.Context, keyID string, created int64, envelope *EnvelopeKeyRecord) (bool, error) { + return true, nil // Simulate successful store +} + +type benchmarkKMS struct{} + +func (k *benchmarkKMS) EncryptKey(ctx context.Context, key []byte) ([]byte, error) { + return internal.GetRandBytes(48), nil // Simulated encrypted key +} + +func (k *benchmarkKMS) DecryptKey(ctx context.Context, encryptedKey []byte) ([]byte, error) { + return internal.GetRandBytes(32), nil // Simulated decrypted key +} + +func (k *benchmarkKMS) Close() error { + return nil +} + +type benchmarkCrypto struct{} + +func (c *benchmarkCrypto) Encrypt(plaintext, key []byte) ([]byte, error) { + // Simulate encryption overhead by doing some work + result := make([]byte, len(plaintext)+16) // Add tag + copy(result, plaintext) + return result, nil +} + +func (c *benchmarkCrypto) Decrypt(ciphertext, key []byte) ([]byte, error) { + // Simulate decryption by returning the original length + if len(ciphertext) < 16 { + return nil, nil + } + return ciphertext[:len(ciphertext)-16], nil +} + +func (c *benchmarkCrypto) GenerateKey() ([]byte, error) { + return internal.GetRandBytes(32), nil +} + +// memStats represents memory statistics for leak detection +type memStats struct { + alloc uint64 + totalAlloc uint64 + sys uint64 + numGC uint32 +} + +// getMemStats returns current memory statistics +func getMemStats() memStats { + runtime.GC() // Force garbage collection for accurate measurements + runtime.GC() // Double GC to ensure cleanup + + var m runtime.MemStats + runtime.ReadMemStats(&m) + + return memStats{ + alloc: m.Alloc, + totalAlloc: m.TotalAlloc, + sys: m.Sys, + numGC: m.NumGC, + } +} + +// checkMemoryLeaks compares memory stats and fails if significant growth is detected +func checkMemoryLeaks(t *testing.T, before, after memStats, testName string) { + var allocGrowthMB float64 + if after.alloc >= before.alloc { + allocGrowthMB = float64(after.alloc-before.alloc) / 1024 / 1024 + } else { + // Handle case where memory decreased (GC freed more than we allocated) + allocGrowthMB = -float64(before.alloc-after.alloc) / 1024 / 1024 + } + + t.Logf("%s Memory Stats:", testName) + t.Logf(" Alloc growth: %.2f MB", allocGrowthMB) + t.Logf(" TotalAlloc growth: %.2f MB", float64(after.totalAlloc-before.totalAlloc)/1024/1024) + t.Logf(" Sys growth: %.2f MB", float64(after.sys-before.sys)/1024/1024) + t.Logf(" GC runs: %d", after.numGC-before.numGC) + + if allocGrowthMB > memLeakToleranceMB { + t.Errorf("Potential memory leak detected in %s: %.2f MB growth (tolerance: %d MB)", + testName, allocGrowthMB, memLeakToleranceMB) + } +} + +// TestKeyCache_MemoryLeaks tests for memory leaks in key cache operations +func TestKeyCache_MemoryLeaks(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T, cache *keyCache) + }{ + { + name: "GetOrLoad_SameKey", + testFunc: func(t *testing.T, cache *keyCache) { + keyMeta := KeyMeta{ID: "leak_test_key", Created: time.Now().Unix()} + + for i := 0; i < memLeakTestIterations; i++ { + key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { + return internal.NewCryptoKeyForTest(meta.Created, false), nil + }) + require.NoError(t, err) + key.Close() + } + }, + }, + { + name: "GetOrLoad_UniqueKeys", + testFunc: func(t *testing.T, cache *keyCache) { + for i := 0; i < memLeakTestIterations; i++ { + keyMeta := KeyMeta{ID: fmt.Sprintf("leak_test_key_%d", i), Created: time.Now().Unix()} + + key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { + return internal.NewCryptoKeyForTest(meta.Created, false), nil + }) + require.NoError(t, err) + key.Close() + } + }, + }, + { + name: "GetOrLoadLatest_SameKey", + testFunc: func(t *testing.T, cache *keyCache) { + keyID := "leak_test_latest_key" + + for i := 0; i < memLeakTestIterations; i++ { + key, err := cache.GetOrLoadLatest(keyID, func(meta KeyMeta) (*internal.CryptoKey, error) { + return internal.NewCryptoKeyForTest(time.Now().Unix(), false), nil + }) + require.NoError(t, err) + key.Close() + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) + defer cache.Close() + + before := getMemStats() + tt.testFunc(t, cache) + after := getMemStats() + + checkMemoryLeaks(t, before, after, fmt.Sprintf("KeyCache_%s", tt.name)) + }) + } +} + +// TestCachedCryptoKey_ReferenceCountingLeaks tests for leaks in reference counting +func TestCachedCryptoKey_ReferenceCountingLeaks(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Increment_Decrement_Cycles", + testFunc: func(t *testing.T) { + for i := 0; i < memLeakTestIterations; i++ { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + + // Simulate reference counting cycles + cachedKey.increment() + cachedKey.increment() + cachedKey.Close() // -1 + cachedKey.Close() // -1 + cachedKey.Close() // -1 (should trigger key.Close()) + } + }, + }, + { + name: "Multiple_References_Same_Key", + testFunc: func(t *testing.T) { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + + for i := 0; i < memLeakTestIterations; i++ { + cachedKey.increment() + cachedKey.Close() + } + + // Final cleanup + cachedKey.Close() + }, + }, + { + name: "Create_Close_Cycle", + testFunc: func(t *testing.T) { + for i := 0; i < memLeakTestIterations; i++ { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + cachedKey.Close() + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := getMemStats() + tt.testFunc(t) + after := getMemStats() + + checkMemoryLeaks(t, before, after, fmt.Sprintf("CachedCryptoKey_%s", tt.name)) + }) + } +} + +// TestSessionFactory_MemoryLeaks tests for memory leaks in session factory operations +func TestSessionFactory_MemoryLeaks(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T, factory *SessionFactory) + }{ + { + name: "GetSession_Close_Cycle", + testFunc: func(t *testing.T, factory *SessionFactory) { + partitionID := "leak_test_partition" + + for i := 0; i < memLeakTestIterations; i++ { + session, err := factory.GetSession(partitionID) + require.NoError(t, err) + session.Close() + } + }, + }, + { + name: "Multiple_Partitions", + testFunc: func(t *testing.T, factory *SessionFactory) { + for i := 0; i < memLeakTestIterations; i++ { + partitionID := fmt.Sprintf("leak_test_partition_%d", i) + session, err := factory.GetSession(partitionID) + require.NoError(t, err) + session.Close() + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "leak_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + before := getMemStats() + tt.testFunc(t, factory) + after := getMemStats() + + checkMemoryLeaks(t, before, after, fmt.Sprintf("SessionFactory_%s", tt.name)) + }) + } +} + +// TestSession_EncryptDecrypt_MemoryLeaks tests for memory leaks in encrypt/decrypt operations +func TestSession_EncryptDecrypt_MemoryLeaks(t *testing.T) { + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "leak_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + session, err := factory.GetSession("leak_test_partition") + require.NoError(t, err) + defer session.Close() + + ctx := context.Background() + payload := internal.GetRandBytes(1024) + + before := getMemStats() + + // Perform many encrypt/decrypt cycles + for i := 0; i < memLeakTestIterations; i++ { + drr, err := session.Encrypt(ctx, payload) + require.NoError(t, err) + + decrypted, err := session.Decrypt(ctx, *drr) + require.NoError(t, err) + require.Equal(t, len(payload), len(decrypted)) + } + + after := getMemStats() + checkMemoryLeaks(t, before, after, "Session_EncryptDecrypt") +} + +// TestGoroutineLeaks tests for goroutine leaks +func TestGoroutineLeaks(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "SessionFactory_Creation_Cleanup", + testFunc: func(t *testing.T) { + for i := 0; i < 100; i++ { // Fewer iterations for goroutine tests + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "goroutine_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + + // Create and close some sessions + session, err := factory.GetSession("test_partition") + require.NoError(t, err) + session.Close() + + factory.Close() + } + }, + }, + { + name: "KeyCache_Concurrent_Access", + testFunc: func(t *testing.T) { + cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) + defer cache.Close() + + keyMeta := KeyMeta{ID: "goroutine_test_key", Created: time.Now().Unix()} + + // Create many concurrent operations + for i := 0; i < 100; i++ { + go func() { + key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { + return internal.NewCryptoKeyForTest(meta.Created, false), nil + }) + if err == nil { + key.Close() + } + }() + } + + // Allow goroutines to complete + time.Sleep(100 * time.Millisecond) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := runtime.NumGoroutine() + tt.testFunc(t) + + // Allow time for goroutines to cleanup + runtime.GC() + time.Sleep(100 * time.Millisecond) + + after := runtime.NumGoroutine() + goroutineGrowth := after - before + + t.Logf("Goroutine growth: %d (before: %d, after: %d)", goroutineGrowth, before, after) + + // Allow some tolerance for background goroutines + if goroutineGrowth > 5 { + t.Errorf("Potential goroutine leak detected in %s: %d new goroutines", tt.name, goroutineGrowth) + } + }) + } +} + +// TestMemoryLeaks_WithCache tests memory leaks when session caching is enabled +func TestMemoryLeaks_WithCache(t *testing.T) { + config := &Config{ + Policy: &CryptoPolicy{ + CacheSessions: true, + SessionCacheMaxSize: 100, + SharedIntermediateKeyCache: true, + }, + Product: "cache_leak_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + before := getMemStats() + + // Create many sessions that should be cached + partitions := make(map[string]bool) + for i := 0; i < memLeakTestIterations; i++ { + partitionID := fmt.Sprintf("cache_test_%d", i%50) // Reuse 50 partitions + partitions[partitionID] = true + + session, err := factory.GetSession(partitionID) + require.NoError(t, err) + session.Close() + } + + after := getMemStats() + + t.Logf("Created sessions for %d unique partitions", len(partitions)) + checkMemoryLeaks(t, before, after, "SessionCache") +} + +// TestMemoryLeaks_LargePayloads tests memory leaks with large payloads +func TestMemoryLeaks_LargePayloads(t *testing.T) { + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "large_payload_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + session, err := factory.GetSession("large_payload_partition") + require.NoError(t, err) + defer session.Close() + + ctx := context.Background() + largePayload := internal.GetRandBytes(64 * 1024) // 64KB + + before := getMemStats() + + // Encrypt/decrypt large payloads multiple times + for i := 0; i < 100; i++ { // Fewer iterations due to large payload size + drr, err := session.Encrypt(ctx, largePayload) + require.NoError(t, err) + + decrypted, err := session.Decrypt(ctx, *drr) + require.NoError(t, err) + require.Equal(t, len(largePayload), len(decrypted)) + } + + after := getMemStats() + checkMemoryLeaks(t, before, after, "LargePayloads") +} + +// TestMemoryLeaks_ReferenceCountingEdgeCases tests edge cases in reference counting +func TestMemoryLeaks_ReferenceCountingEdgeCases(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Double_Close", + testFunc: func(t *testing.T) { + for i := 0; i < memLeakTestIterations/10; i++ { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + + // Close multiple times (should be safe) + cachedKey.Close() + cachedKey.Close() // Should be no-op + cachedKey.Close() // Should be no-op + } + }, + }, + { + name: "Increment_After_Close", + testFunc: func(t *testing.T) { + for i := 0; i < memLeakTestIterations/10; i++ { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + + cachedKey.Close() // Ref count goes to 0, key is closed + + // This should still work but key is already closed + // This tests that we don't leak memory even in edge cases + cachedKey.increment() + cachedKey.Close() + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := getMemStats() + tt.testFunc(t) + after := getMemStats() + + checkMemoryLeaks(t, before, after, fmt.Sprintf("ReferenceCountingEdgeCase_%s", tt.name)) + }) + } +} + +// BenchmarkMemoryLeaks_SessionOperations provides benchmarks that can detect memory leaks over time +func BenchmarkMemoryLeaks_SessionOperations(b *testing.B) { + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "benchmark_leak_test", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + session, err := factory.GetSession("benchmark_partition") + require.NoError(b, err) + defer session.Close() + + ctx := context.Background() + payload := internal.GetRandBytes(1024) + + b.ResetTimer() + + for i := 0; i < b.N; i++ { + drr, err := session.Encrypt(ctx, payload) + if err != nil { + b.Fatal(err) + } + + _, err = session.Decrypt(ctx, *drr) + if err != nil { + b.Fatal(err) + } + } +} \ No newline at end of file diff --git a/go/appencryption/memory_leak_validation_test.go b/go/appencryption/memory_leak_validation_test.go new file mode 100644 index 000000000..cec0c1184 --- /dev/null +++ b/go/appencryption/memory_leak_validation_test.go @@ -0,0 +1,208 @@ +package appencryption + +import ( + "fmt" + "runtime" + "testing" + "time" + + "github.com/godaddy/asherah/go/appencryption/internal" + "github.com/stretchr/testify/require" +) + +// This file contains tests that intentionally create memory leaks to validate +// that our memory leak detection tests actually work and can catch real leaks. +// These tests are expected to fail when leak detection is working properly. + +// TestMemoryLeakDetection_ValidationTest intentionally creates memory leaks to test our detection +func TestMemoryLeakDetection_ValidationTest(t *testing.T) { + t.Skip("This test intentionally creates memory leaks and should only be run manually to validate leak detection") + + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "Intentional_Key_Leak", + testFunc: func(t *testing.T) { + // Intentionally leak cached crypto keys by not closing them + var leakedKeys []*cachedCryptoKey + + for i := 0; i < 1000; i++ { + key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) + cachedKey := newCachedCryptoKey(key) + leakedKeys = append(leakedKeys, cachedKey) + // Intentionally NOT calling cachedKey.Close() + } + + // Keep reference to prevent GC + _ = leakedKeys + }, + }, + { + name: "Intentional_Session_Leak", + testFunc: func(t *testing.T) { + config := &Config{ + Policy: NewCryptoPolicy(), + Product: "leak_validation", + Service: "test", + } + + factory := NewSessionFactory( + config, + &benchmarkMetastore{}, + &benchmarkKMS{}, + &benchmarkCrypto{}, + WithSecretFactory(memLeakSecretFactory), + ) + defer factory.Close() + + // Intentionally create sessions and not close them + var leakedSessions []*Session + + for i := 0; i < 100; i++ { + session, err := factory.GetSession("leak_test") + require.NoError(t, err) + leakedSessions = append(leakedSessions, session) + // Intentionally NOT calling session.Close() + } + + // Keep reference to prevent GC + _ = leakedSessions + }, + }, + { + name: "Intentional_Goroutine_Leak", + testFunc: func(t *testing.T) { + // Create goroutines that never exit + for i := 0; i < 10; i++ { + go func() { + // Infinite loop - goroutine will never exit + for { + time.Sleep(1 * time.Hour) + } + }() + } + + // Give goroutines time to start + time.Sleep(10 * time.Millisecond) + }, + }, + { + name: "Intentional_Memory_Growth", + testFunc: func(t *testing.T) { + // Allocate large amounts of memory and keep references + var leakedMemory [][]byte + + for i := 0; i < 100; i++ { + // Allocate 1MB chunks + chunk := make([]byte, 1024*1024) + leakedMemory = append(leakedMemory, chunk) + } + + // Keep reference to prevent GC + _ = leakedMemory + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := getMemStats() + beforeGoroutines := runtime.NumGoroutine() + + tt.testFunc(t) + + after := getMemStats() + afterGoroutines := runtime.NumGoroutine() + + // This test should fail if leak detection is working + checkMemoryLeaks(t, before, after, fmt.Sprintf("ValidationTest_%s", tt.name)) + + goroutineGrowth := afterGoroutines - beforeGoroutines + if goroutineGrowth > 5 { + t.Errorf("Goroutine leak detected (expected): %d new goroutines", goroutineGrowth) + } + }) + } +} + +// TestMemoryLeakDetection_EdgeCases tests edge cases that might cause false positives +func TestMemoryLeakDetection_EdgeCases(t *testing.T) { + tests := []struct { + name string + testFunc func(t *testing.T) + }{ + { + name: "GC_Timing_Sensitive", + testFunc: func(t *testing.T) { + // Create temporary allocations that should be GC'd + for i := 0; i < 1000; i++ { + temp := make([]byte, 1024) + _ = temp // Use it briefly then let it go out of scope + } + }, + }, + { + name: "Background_Goroutines", + testFunc: func(t *testing.T) { + // Create goroutines that clean themselves up + done := make(chan bool, 10) + + for i := 0; i < 10; i++ { + go func() { + time.Sleep(10 * time.Millisecond) + done <- true + }() + } + + // Wait for all goroutines to finish + for i := 0; i < 10; i++ { + <-done + } + }, + }, + { + name: "Normal_Key_Operations", + testFunc: func(t *testing.T) { + // Perform normal operations that should not leak + cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) + defer cache.Close() + + keyMeta := KeyMeta{ID: "edge_case_key", Created: time.Now().Unix()} + + for i := 0; i < 100; i++ { + key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { + return internal.NewCryptoKeyForTest(meta.Created, false), nil + }) + require.NoError(t, err) + key.Close() + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + before := getMemStats() + beforeGoroutines := runtime.NumGoroutine() + + tt.testFunc(t) + + // Allow time for cleanup + runtime.GC() + time.Sleep(50 * time.Millisecond) + + after := getMemStats() + afterGoroutines := runtime.NumGoroutine() + + // These should pass (no leaks detected) + checkMemoryLeaks(t, before, after, fmt.Sprintf("EdgeCase_%s", tt.name)) + + goroutineGrowth := afterGoroutines - beforeGoroutines + if goroutineGrowth > 5 { + t.Errorf("Unexpected goroutine growth in edge case: %d new goroutines", goroutineGrowth) + } + }) + } +} \ No newline at end of file From 09b24b3a1e68f09623f7373cdf91dbdec05405fc Mon Sep 17 00:00:00 2001 From: Jeremiah Gowdy Date: Sun, 3 Aug 2025 18:17:10 -0700 Subject: [PATCH 2/2] Fix linting issues in memory leak tests - Fix gci import ordering issues - Fix gofumpt formatting for var declarations - Split long functions to satisfy funlen linter - Extract helper functions for test execution - Add nolint directive for test case definition function --- go/appencryption/go.work | 2 + go/appencryption/go.work.sum | 7 +- go/appencryption/memory_leak_test.go | 164 +++++++++--------- .../memory_leak_validation_test.go | 102 ++++++----- 4 files changed, 145 insertions(+), 130 deletions(-) diff --git a/go/appencryption/go.work b/go/appencryption/go.work index f928d8d95..0a00f043e 100644 --- a/go/appencryption/go.work +++ b/go/appencryption/go.work @@ -1,5 +1,7 @@ go 1.23.0 +toolchain go1.22.5 + use ( . ./cmd/example diff --git a/go/appencryption/go.work.sum b/go/appencryption/go.work.sum index 569518e7b..03a93143a 100644 --- a/go/appencryption/go.work.sum +++ b/go/appencryption/go.work.sum @@ -227,7 +227,6 @@ github.com/containerd/typeurl v1.0.2 h1:Chlt8zIieDbzQFzXzAeBEF92KhExuE4p9p92/QmY github.com/containerd/typeurl v1.0.2/go.mod h1:9trJWW2sRlGub4wZJRTW83VtbOLS6hwcDZXTn6oPz9s= github.com/containerd/typeurl/v2 v2.1.1 h1:3Q4Pt7i8nYwy2KmQWIw2+1hTvwTE/6w9FqcttATPO/4= github.com/containerd/typeurl/v2 v2.1.1/go.mod h1:IDp2JFvbwZ31H8dQbEIY7sDl2L3o3HZj1hsSQlywkQ0= -github.com/containerd/typeurl/v2 v2.2.0/go.mod h1:8XOOxnyatxSWuG8OfsZXVnAF4iZfedjS/8UHSPJnX4g= github.com/containerd/zfs v1.0.0 h1:cXLJbx+4Jj7rNsTiqVfm6i+RNLx6FFA2fMmDlEf+Wm8= github.com/containerd/zfs v1.1.0 h1:n7OZ7jZumLIqNJqXrEc/paBM840mORnmGdJDmAmJZHM= github.com/containerd/zfs v1.1.0/go.mod h1:oZF9wBnrnQjpWLaPKEinrx3TQ9a+W/RJO7Zb41d8YLE= @@ -324,7 +323,6 @@ github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= -github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= @@ -410,11 +408,8 @@ github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg= github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc= github.com/moby/spdystream v0.2.0 h1:cjW1zVyyoiM0T7b6UoySUFqzXMoqRckQtXwGPiBhOM8= github.com/moby/spdystream v0.2.0/go.mod h1:f7i0iNDQJ059oMTcWxx8MA/zKFIuD/lY+0GqbN2Wy8c= -github.com/moby/sys/mount v0.3.4/go.mod h1:KcQJMbQdJHPlq5lcYT+/CjatWM4PuxKe+XLSVS4J6Os= github.com/moby/sys/mountinfo v0.6.2 h1:BzJjoreD5BMFNmD9Rus6gdd1pLuecOFPt8wC+Vygl78= github.com/moby/sys/mountinfo v0.6.2/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI= -github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4= -github.com/moby/sys/reexec v0.1.0/go.mod h1:EqjBg8F3X7iZe5pU6nRZnYCMUTXoxsjiIfHup5wYIN8= github.com/moby/sys/signal v0.6.0 h1:aDpY94H8VlhTGa9sNYUFCFsMZIUh5wm0B6XkIoJj/iY= github.com/moby/sys/signal v0.7.0 h1:25RW3d5TnQEoKvRbEKUGay6DCQ46IxAVTT9CUMgmsSI= github.com/moby/sys/signal v0.7.0/go.mod h1:GQ6ObYZfqacOwTtlXvcmh9A26dVRul/hbOZn88Kg8Tg= @@ -472,7 +467,6 @@ github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0 github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8 h1:2c1EFnZHIPCW8qKWgHMH/fX2PkSabFc5mrVzfUNdg5U= -github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY= github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww= github.com/sclevine/spec v1.2.0 h1:1Jwdf9jSfDl9NVmt8ndHqbTZ7XCCPbh1jI3hkDBHVYA= github.com/seccomp/libseccomp-golang v0.9.2-0.20220502022130-f33da4d89646 h1:RpforrEYXWkmGwJHIGnLZ3tTWStkjVVstwzNGqxX2Ds= @@ -575,6 +569,7 @@ golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.31.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA= +golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0= golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/term v0.15.0/go.mod h1:BDl952bC7+uMoWR75FIrCDx79TPU9oHkTZ9yRbYOrX0= diff --git a/go/appencryption/memory_leak_test.go b/go/appencryption/memory_leak_test.go index ecb034b84..91a069e83 100644 --- a/go/appencryption/memory_leak_test.go +++ b/go/appencryption/memory_leak_test.go @@ -7,9 +7,10 @@ import ( "testing" "time" - "github.com/godaddy/asherah/go/appencryption/internal" "github.com/godaddy/asherah/go/securememory/memguard" "github.com/stretchr/testify/require" + + "github.com/godaddy/asherah/go/appencryption/internal" ) // Memory leak detection tests for Asherah Go implementation @@ -39,9 +40,7 @@ const ( memLeakToleranceMB = 5 // MB tolerance for memory growth ) -var ( - memLeakSecretFactory = new(memguard.SecretFactory) -) +var memLeakSecretFactory = new(memguard.SecretFactory) // Create minimal test implementations to avoid import cycles @@ -106,10 +105,10 @@ type memStats struct { func getMemStats() memStats { runtime.GC() // Force garbage collection for accurate measurements runtime.GC() // Double GC to ensure cleanup - + var m runtime.MemStats runtime.ReadMemStats(&m) - + return memStats{ alloc: m.Alloc, totalAlloc: m.TotalAlloc, @@ -127,15 +126,15 @@ func checkMemoryLeaks(t *testing.T, before, after memStats, testName string) { // Handle case where memory decreased (GC freed more than we allocated) allocGrowthMB = -float64(before.alloc-after.alloc) / 1024 / 1024 } - + t.Logf("%s Memory Stats:", testName) t.Logf(" Alloc growth: %.2f MB", allocGrowthMB) t.Logf(" TotalAlloc growth: %.2f MB", float64(after.totalAlloc-before.totalAlloc)/1024/1024) t.Logf(" Sys growth: %.2f MB", float64(after.sys-before.sys)/1024/1024) t.Logf(" GC runs: %d", after.numGC-before.numGC) - + if allocGrowthMB > memLeakToleranceMB { - t.Errorf("Potential memory leak detected in %s: %.2f MB growth (tolerance: %d MB)", + t.Errorf("Potential memory leak detected in %s: %.2f MB growth (tolerance: %d MB)", testName, allocGrowthMB, memLeakToleranceMB) } } @@ -150,7 +149,7 @@ func TestKeyCache_MemoryLeaks(t *testing.T) { name: "GetOrLoad_SameKey", testFunc: func(t *testing.T, cache *keyCache) { keyMeta := KeyMeta{ID: "leak_test_key", Created: time.Now().Unix()} - + for i := 0; i < memLeakTestIterations; i++ { key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { return internal.NewCryptoKeyForTest(meta.Created, false), nil @@ -165,7 +164,7 @@ func TestKeyCache_MemoryLeaks(t *testing.T) { testFunc: func(t *testing.T, cache *keyCache) { for i := 0; i < memLeakTestIterations; i++ { keyMeta := KeyMeta{ID: fmt.Sprintf("leak_test_key_%d", i), Created: time.Now().Unix()} - + key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { return internal.NewCryptoKeyForTest(meta.Created, false), nil }) @@ -178,7 +177,7 @@ func TestKeyCache_MemoryLeaks(t *testing.T) { name: "GetOrLoadLatest_SameKey", testFunc: func(t *testing.T, cache *keyCache) { keyID := "leak_test_latest_key" - + for i := 0; i < memLeakTestIterations; i++ { key, err := cache.GetOrLoadLatest(keyID, func(meta KeyMeta) (*internal.CryptoKey, error) { return internal.NewCryptoKeyForTest(time.Now().Unix(), false), nil @@ -189,16 +188,16 @@ func TestKeyCache_MemoryLeaks(t *testing.T) { }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) defer cache.Close() - + before := getMemStats() tt.testFunc(t, cache) after := getMemStats() - + checkMemoryLeaks(t, before, after, fmt.Sprintf("KeyCache_%s", tt.name)) }) } @@ -216,7 +215,7 @@ func TestCachedCryptoKey_ReferenceCountingLeaks(t *testing.T) { for i := 0; i < memLeakTestIterations; i++ { key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) cachedKey := newCachedCryptoKey(key) - + // Simulate reference counting cycles cachedKey.increment() cachedKey.increment() @@ -231,12 +230,12 @@ func TestCachedCryptoKey_ReferenceCountingLeaks(t *testing.T) { testFunc: func(t *testing.T) { key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) cachedKey := newCachedCryptoKey(key) - + for i := 0; i < memLeakTestIterations; i++ { cachedKey.increment() cachedKey.Close() } - + // Final cleanup cachedKey.Close() }, @@ -252,13 +251,13 @@ func TestCachedCryptoKey_ReferenceCountingLeaks(t *testing.T) { }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { before := getMemStats() tt.testFunc(t) after := getMemStats() - + checkMemoryLeaks(t, before, after, fmt.Sprintf("CachedCryptoKey_%s", tt.name)) }) } @@ -274,7 +273,7 @@ func TestSessionFactory_MemoryLeaks(t *testing.T) { name: "GetSession_Close_Cycle", testFunc: func(t *testing.T, factory *SessionFactory) { partitionID := "leak_test_partition" - + for i := 0; i < memLeakTestIterations; i++ { session, err := factory.GetSession(partitionID) require.NoError(t, err) @@ -294,7 +293,7 @@ func TestSessionFactory_MemoryLeaks(t *testing.T) { }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { config := &Config{ @@ -302,7 +301,7 @@ func TestSessionFactory_MemoryLeaks(t *testing.T) { Product: "leak_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -311,11 +310,11 @@ func TestSessionFactory_MemoryLeaks(t *testing.T) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + before := getMemStats() tt.testFunc(t, factory) after := getMemStats() - + checkMemoryLeaks(t, before, after, fmt.Sprintf("SessionFactory_%s", tt.name)) }) } @@ -328,7 +327,7 @@ func TestSession_EncryptDecrypt_MemoryLeaks(t *testing.T) { Product: "leak_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -337,30 +336,50 @@ func TestSession_EncryptDecrypt_MemoryLeaks(t *testing.T) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + session, err := factory.GetSession("leak_test_partition") require.NoError(t, err) defer session.Close() - + ctx := context.Background() payload := internal.GetRandBytes(1024) - + before := getMemStats() - + // Perform many encrypt/decrypt cycles for i := 0; i < memLeakTestIterations; i++ { drr, err := session.Encrypt(ctx, payload) require.NoError(t, err) - + decrypted, err := session.Decrypt(ctx, *drr) require.NoError(t, err) require.Equal(t, len(payload), len(decrypted)) } - + after := getMemStats() checkMemoryLeaks(t, before, after, "Session_EncryptDecrypt") } +// testGoroutineLeaks runs a test and checks for goroutine leaks +func testGoroutineLeaks(t *testing.T, name string, testFunc func(t *testing.T)) { + before := runtime.NumGoroutine() + testFunc(t) + + // Allow time for goroutines to cleanup + runtime.GC() + time.Sleep(100 * time.Millisecond) + + after := runtime.NumGoroutine() + goroutineGrowth := after - before + + t.Logf("Goroutine growth: %d (before: %d, after: %d)", goroutineGrowth, before, after) + + // Allow some tolerance for background goroutines + if goroutineGrowth > 5 { + t.Errorf("Potential goroutine leak detected in %s: %d new goroutines", name, goroutineGrowth) + } +} + // TestGoroutineLeaks tests for goroutine leaks func TestGoroutineLeaks(t *testing.T) { tests := []struct { @@ -376,7 +395,7 @@ func TestGoroutineLeaks(t *testing.T) { Product: "goroutine_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -384,12 +403,12 @@ func TestGoroutineLeaks(t *testing.T) { &benchmarkCrypto{}, WithSecretFactory(memLeakSecretFactory), ) - + // Create and close some sessions session, err := factory.GetSession("test_partition") require.NoError(t, err) session.Close() - + factory.Close() } }, @@ -399,9 +418,9 @@ func TestGoroutineLeaks(t *testing.T) { testFunc: func(t *testing.T) { cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) defer cache.Close() - + keyMeta := KeyMeta{ID: "goroutine_test_key", Created: time.Now().Unix()} - + // Create many concurrent operations for i := 0; i < 100; i++ { go func() { @@ -413,31 +432,16 @@ func TestGoroutineLeaks(t *testing.T) { } }() } - + // Allow goroutines to complete time.Sleep(100 * time.Millisecond) }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - before := runtime.NumGoroutine() - tt.testFunc(t) - - // Allow time for goroutines to cleanup - runtime.GC() - time.Sleep(100 * time.Millisecond) - - after := runtime.NumGoroutine() - goroutineGrowth := after - before - - t.Logf("Goroutine growth: %d (before: %d, after: %d)", goroutineGrowth, before, after) - - // Allow some tolerance for background goroutines - if goroutineGrowth > 5 { - t.Errorf("Potential goroutine leak detected in %s: %d new goroutines", tt.name, goroutineGrowth) - } + testGoroutineLeaks(t, tt.name, tt.testFunc) }) } } @@ -453,7 +457,7 @@ func TestMemoryLeaks_WithCache(t *testing.T) { Product: "cache_leak_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -462,22 +466,22 @@ func TestMemoryLeaks_WithCache(t *testing.T) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + before := getMemStats() - + // Create many sessions that should be cached partitions := make(map[string]bool) for i := 0; i < memLeakTestIterations; i++ { partitionID := fmt.Sprintf("cache_test_%d", i%50) // Reuse 50 partitions partitions[partitionID] = true - + session, err := factory.GetSession(partitionID) require.NoError(t, err) session.Close() } - + after := getMemStats() - + t.Logf("Created sessions for %d unique partitions", len(partitions)) checkMemoryLeaks(t, before, after, "SessionCache") } @@ -489,7 +493,7 @@ func TestMemoryLeaks_LargePayloads(t *testing.T) { Product: "large_payload_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -498,26 +502,26 @@ func TestMemoryLeaks_LargePayloads(t *testing.T) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + session, err := factory.GetSession("large_payload_partition") require.NoError(t, err) defer session.Close() - + ctx := context.Background() largePayload := internal.GetRandBytes(64 * 1024) // 64KB - + before := getMemStats() - + // Encrypt/decrypt large payloads multiple times for i := 0; i < 100; i++ { // Fewer iterations due to large payload size drr, err := session.Encrypt(ctx, largePayload) require.NoError(t, err) - + decrypted, err := session.Decrypt(ctx, *drr) require.NoError(t, err) require.Equal(t, len(largePayload), len(decrypted)) } - + after := getMemStats() checkMemoryLeaks(t, before, after, "LargePayloads") } @@ -534,7 +538,7 @@ func TestMemoryLeaks_ReferenceCountingEdgeCases(t *testing.T) { for i := 0; i < memLeakTestIterations/10; i++ { key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) cachedKey := newCachedCryptoKey(key) - + // Close multiple times (should be safe) cachedKey.Close() cachedKey.Close() // Should be no-op @@ -548,9 +552,9 @@ func TestMemoryLeaks_ReferenceCountingEdgeCases(t *testing.T) { for i := 0; i < memLeakTestIterations/10; i++ { key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) cachedKey := newCachedCryptoKey(key) - + cachedKey.Close() // Ref count goes to 0, key is closed - + // This should still work but key is already closed // This tests that we don't leak memory even in edge cases cachedKey.increment() @@ -559,13 +563,13 @@ func TestMemoryLeaks_ReferenceCountingEdgeCases(t *testing.T) { }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { before := getMemStats() tt.testFunc(t) after := getMemStats() - + checkMemoryLeaks(t, before, after, fmt.Sprintf("ReferenceCountingEdgeCase_%s", tt.name)) }) } @@ -578,7 +582,7 @@ func BenchmarkMemoryLeaks_SessionOperations(b *testing.B) { Product: "benchmark_leak_test", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -587,25 +591,25 @@ func BenchmarkMemoryLeaks_SessionOperations(b *testing.B) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + session, err := factory.GetSession("benchmark_partition") require.NoError(b, err) defer session.Close() - + ctx := context.Background() payload := internal.GetRandBytes(1024) - + b.ResetTimer() - + for i := 0; i < b.N; i++ { drr, err := session.Encrypt(ctx, payload) if err != nil { b.Fatal(err) } - + _, err = session.Decrypt(ctx, *drr) if err != nil { b.Fatal(err) } } -} \ No newline at end of file +} diff --git a/go/appencryption/memory_leak_validation_test.go b/go/appencryption/memory_leak_validation_test.go index cec0c1184..aaf231337 100644 --- a/go/appencryption/memory_leak_validation_test.go +++ b/go/appencryption/memory_leak_validation_test.go @@ -6,19 +6,42 @@ import ( "testing" "time" - "github.com/godaddy/asherah/go/appencryption/internal" "github.com/stretchr/testify/require" + + "github.com/godaddy/asherah/go/appencryption/internal" ) // This file contains tests that intentionally create memory leaks to validate // that our memory leak detection tests actually work and can catch real leaks. // These tests are expected to fail when leak detection is working properly. -// TestMemoryLeakDetection_ValidationTest intentionally creates memory leaks to test our detection -func TestMemoryLeakDetection_ValidationTest(t *testing.T) { - t.Skip("This test intentionally creates memory leaks and should only be run manually to validate leak detection") - - tests := []struct { +// runLeakValidationTest runs a validation test that's expected to detect leaks +func runLeakValidationTest(t *testing.T, name string, testFunc func(t *testing.T)) { + before := getMemStats() + beforeGoroutines := runtime.NumGoroutine() + + testFunc(t) + + after := getMemStats() + afterGoroutines := runtime.NumGoroutine() + + // This test should fail if leak detection is working + checkMemoryLeaks(t, before, after, fmt.Sprintf("ValidationTest_%s", name)) + + goroutineGrowth := afterGoroutines - beforeGoroutines + if goroutineGrowth > 5 { + t.Errorf("Goroutine leak detected (expected): %d new goroutines", goroutineGrowth) + } +} + +// getLeakValidationTests returns test cases that intentionally create memory leaks +// +//nolint:funlen // Test case definitions +func getLeakValidationTests() []struct { + name string + testFunc func(t *testing.T) +} { + return []struct { name string testFunc func(t *testing.T) }{ @@ -27,14 +50,14 @@ func TestMemoryLeakDetection_ValidationTest(t *testing.T) { testFunc: func(t *testing.T) { // Intentionally leak cached crypto keys by not closing them var leakedKeys []*cachedCryptoKey - + for i := 0; i < 1000; i++ { key := internal.NewCryptoKeyForTest(time.Now().Unix(), false) cachedKey := newCachedCryptoKey(key) leakedKeys = append(leakedKeys, cachedKey) // Intentionally NOT calling cachedKey.Close() } - + // Keep reference to prevent GC _ = leakedKeys }, @@ -47,7 +70,7 @@ func TestMemoryLeakDetection_ValidationTest(t *testing.T) { Product: "leak_validation", Service: "test", } - + factory := NewSessionFactory( config, &benchmarkMetastore{}, @@ -56,17 +79,17 @@ func TestMemoryLeakDetection_ValidationTest(t *testing.T) { WithSecretFactory(memLeakSecretFactory), ) defer factory.Close() - + // Intentionally create sessions and not close them var leakedSessions []*Session - + for i := 0; i < 100; i++ { session, err := factory.GetSession("leak_test") require.NoError(t, err) leakedSessions = append(leakedSessions, session) // Intentionally NOT calling session.Close() } - + // Keep reference to prevent GC _ = leakedSessions }, @@ -83,7 +106,7 @@ func TestMemoryLeakDetection_ValidationTest(t *testing.T) { } }() } - + // Give goroutines time to start time.Sleep(10 * time.Millisecond) }, @@ -93,36 +116,27 @@ func TestMemoryLeakDetection_ValidationTest(t *testing.T) { testFunc: func(t *testing.T) { // Allocate large amounts of memory and keep references var leakedMemory [][]byte - + for i := 0; i < 100; i++ { // Allocate 1MB chunks chunk := make([]byte, 1024*1024) leakedMemory = append(leakedMemory, chunk) } - + // Keep reference to prevent GC _ = leakedMemory }, }, } - - for _, tt := range tests { +} + +// TestMemoryLeakDetection_ValidationTest intentionally creates memory leaks to test our detection +func TestMemoryLeakDetection_ValidationTest(t *testing.T) { + t.Skip("This test intentionally creates memory leaks and should only be run manually to validate leak detection") + + for _, tt := range getLeakValidationTests() { t.Run(tt.name, func(t *testing.T) { - before := getMemStats() - beforeGoroutines := runtime.NumGoroutine() - - tt.testFunc(t) - - after := getMemStats() - afterGoroutines := runtime.NumGoroutine() - - // This test should fail if leak detection is working - checkMemoryLeaks(t, before, after, fmt.Sprintf("ValidationTest_%s", tt.name)) - - goroutineGrowth := afterGoroutines - beforeGoroutines - if goroutineGrowth > 5 { - t.Errorf("Goroutine leak detected (expected): %d new goroutines", goroutineGrowth) - } + runLeakValidationTest(t, tt.name, tt.testFunc) }) } } @@ -148,14 +162,14 @@ func TestMemoryLeakDetection_EdgeCases(t *testing.T) { testFunc: func(t *testing.T) { // Create goroutines that clean themselves up done := make(chan bool, 10) - + for i := 0; i < 10; i++ { go func() { time.Sleep(10 * time.Millisecond) done <- true }() } - + // Wait for all goroutines to finish for i := 0; i < 10; i++ { <-done @@ -168,9 +182,9 @@ func TestMemoryLeakDetection_EdgeCases(t *testing.T) { // Perform normal operations that should not leak cache := newKeyCache(CacheTypeIntermediateKeys, NewCryptoPolicy()) defer cache.Close() - + keyMeta := KeyMeta{ID: "edge_case_key", Created: time.Now().Unix()} - + for i := 0; i < 100; i++ { key, err := cache.GetOrLoad(keyMeta, func(meta KeyMeta) (*internal.CryptoKey, error) { return internal.NewCryptoKeyForTest(meta.Created, false), nil @@ -181,28 +195,28 @@ func TestMemoryLeakDetection_EdgeCases(t *testing.T) { }, }, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { before := getMemStats() beforeGoroutines := runtime.NumGoroutine() - + tt.testFunc(t) - + // Allow time for cleanup runtime.GC() - time.Sleep(50 * time.Millisecond) - + time.Sleep(50 * time.Millisecond) + after := getMemStats() afterGoroutines := runtime.NumGoroutine() - + // These should pass (no leaks detected) checkMemoryLeaks(t, before, after, fmt.Sprintf("EdgeCase_%s", tt.name)) - + goroutineGrowth := afterGoroutines - beforeGoroutines if goroutineGrowth > 5 { t.Errorf("Unexpected goroutine growth in edge case: %d new goroutines", goroutineGrowth) } }) } -} \ No newline at end of file +}