From e290a0d476bc60e335d6dfa7a51ed2d7d95de75c Mon Sep 17 00:00:00 2001 From: Jin Hai Date: Mon, 3 Aug 2026 17:39:47 +0800 Subject: [PATCH] Go: fix resource leak (#17733) Signed-off-by: Jin Hai --- internal/dao/compilation_template_seed.go | 2 +- internal/storage/minio.go | 12 ++++++++---- internal/storage/oss.go | 19 +++++++++++++----- internal/storage/s3.go | 12 ++++++++---- internal/storage/storage_factory.go | 24 +++++++++++------------ 5 files changed, 43 insertions(+), 26 deletions(-) diff --git a/internal/dao/compilation_template_seed.go b/internal/dao/compilation_template_seed.go index 68c3bdab08..055093ee4f 100644 --- a/internal/dao/compilation_template_seed.go +++ b/internal/dao/compilation_template_seed.go @@ -73,7 +73,7 @@ func builtinTemplateID(kind string) string { // resolvable for every tenant. func SeedBuiltinCompilationTemplates(ctx context.Context, db *gorm.DB) error { if err := SeedBuiltinCompilationTemplatesForTenant(ctx, db, ""); err != nil { - common.Warn("failed to seed built-in compilation templates", zap.Error(err)) + common.Warn("Failed to seed built-in compilation templates", zap.Error(err)) return err } return nil diff --git a/internal/storage/minio.go b/internal/storage/minio.go index bba8500ec4..53626e41ed 100644 --- a/internal/storage/minio.go +++ b/internal/storage/minio.go @@ -198,14 +198,18 @@ func (m *MinioStorage) Get(ctx context.Context, bucket, fnm string, tenantID ... } continue } - defer obj.Close() - buf := new(bytes.Buffer) - if _, err = buf.ReadFrom(obj); err != nil { + + readErr := func() error { + defer obj.Close() + _, err = buf.ReadFrom(obj) + return err + }() + if readErr != nil { if ctxErr := ctx.Err(); ctxErr != nil { return nil, ctxErr } - common.Warn("failed to read object data", zap.String("bucket", bucket), zap.String("key", fnm), zap.Error(err)) + common.Error("failed to read object data", err, zap.String("bucket", bucket), zap.String("key", fnm)) m.reconnect() if err = sleepOrAbort(ctx, time.Second); err != nil { return nil, err diff --git a/internal/storage/oss.go b/internal/storage/oss.go index d5ebc869e0..86072c90ec 100644 --- a/internal/storage/oss.go +++ b/internal/storage/oss.go @@ -213,14 +213,18 @@ func (o *OSSStorage) Get(ctx context.Context, bucket, fnm string, tenantID ...st } continue } - defer result.Body.Close() - buf := new(bytes.Buffer) - if _, err = buf.ReadFrom(result.Body); err != nil { + + readErr := func() error { + defer result.Body.Close() + _, err = buf.ReadFrom(result.Body) + return err + }() + if readErr != nil { if ctxErr := ctx.Err(); ctxErr != nil { return nil, ctxErr } - common.Error("Failed to read object data", err, zap.String("bucket", bucket), zap.String("key", fnm)) + common.Error("Failed to read object data", readErr, zap.String("bucket", bucket), zap.String("key", fnm)) o.reconnect(ctx) if err = sleepOrAbort(ctx, time.Second); err != nil { return nil, err @@ -299,9 +303,14 @@ func (o *OSSStorage) GetPresignedURL(ctx context.Context, bucket, fnm string, ex Key: aws.String(fnm), }, s3.WithPresignExpires(expires)) if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } common.Error("Failed to generate presigned URL", err, zap.String("bucket", bucket), zap.String("key", fnm)) o.reconnect(ctx) - time.Sleep(time.Second) + if err = sleepOrAbort(ctx, time.Second); err != nil { + return "", err + } continue } diff --git a/internal/storage/s3.go b/internal/storage/s3.go index c146e0f0cc..78f4c376e3 100644 --- a/internal/storage/s3.go +++ b/internal/storage/s3.go @@ -216,11 +216,15 @@ func (s *S3Storage) Get(ctx context.Context, bucket, fnm string, tenantID ...str time.Sleep(time.Second) continue } - defer result.Body.Close() - buf := new(bytes.Buffer) - if _, err = buf.ReadFrom(result.Body); err != nil { - common.Error("Failed to read object data", err, zap.String("bucket", bucket), zap.String("key", fnm), zap.Error(err)) + + readErr := func() error { + defer result.Body.Close() + _, err = buf.ReadFrom(result.Body) + return err + }() + if readErr != nil { + common.Error("Failed to read object data", readErr, zap.String("bucket", bucket), zap.String("key", fnm), zap.Error(readErr)) s.reconnect(ctx) time.Sleep(time.Second) continue diff --git a/internal/storage/storage_factory.go b/internal/storage/storage_factory.go index 4fc54a29a6..5aabe0f293 100644 --- a/internal/storage/storage_factory.go +++ b/internal/storage/storage_factory.go @@ -26,20 +26,20 @@ import ( ) var ( - globalFactory *StorageFactory + globalFactory *Factory once sync.Once ) -// StorageFactory creates storage instances based on configuration -type StorageFactory struct { +// Factory creates storage instances based on configuration +type Factory struct { storage Storage mu sync.RWMutex } // GetStorageFactory returns the singleton storage factory instance -func GetStorageFactory() *StorageFactory { +func GetStorageFactory() *Factory { once.Do(func() { - globalFactory = &StorageFactory{} + globalFactory = &Factory{} }) return globalFactory } @@ -67,7 +67,7 @@ func CloseStorage() error { return factory.storage.Close() } -func (f *StorageFactory) initStorage(ctx context.Context) error { +func (f *Factory) initStorage(ctx context.Context) error { globalConfig := server.GetConfig() switch globalConfig.StorageEngineType() { case "minio": @@ -83,7 +83,7 @@ func (f *StorageFactory) initStorage(ctx context.Context) error { } } -func (f *StorageFactory) initMinio() error { +func (f *Factory) initMinio() error { globalConfig := server.GetConfig() storage, err := NewMinioStorage(globalConfig.GetMinioConfig()) if err != nil { @@ -97,7 +97,7 @@ func (f *StorageFactory) initMinio() error { return nil } -func (f *StorageFactory) initS3(ctx context.Context) error { +func (f *Factory) initS3(ctx context.Context) error { globalConfig := server.GetConfig() storage, err := NewS3Storage(ctx, globalConfig.GetS3Config()) if err != nil { @@ -111,7 +111,7 @@ func (f *StorageFactory) initS3(ctx context.Context) error { return nil } -func (f *StorageFactory) initOSS(ctx context.Context) error { +func (f *Factory) initOSS(ctx context.Context) error { globalConfig := server.GetConfig() storage, err := NewOSSStorage(ctx, globalConfig.GetOSSConfig()) if err != nil { @@ -125,7 +125,7 @@ func (f *StorageFactory) initOSS(ctx context.Context) error { return nil } -func (f *StorageFactory) initGCS(ctx context.Context) error { +func (f *Factory) initGCS(ctx context.Context) error { globalConfig := server.GetConfig() storage, err := NewGCSStorage(ctx, globalConfig.GetGCSConfig()) if err != nil { @@ -140,14 +140,14 @@ func (f *StorageFactory) initGCS(ctx context.Context) error { } // GetStorage returns the current storage instance -func (f *StorageFactory) GetStorage() Storage { +func (f *Factory) GetStorage() Storage { f.mu.RLock() defer f.mu.RUnlock() return f.storage } // SetStorage sets the storage instance (useful for testing) -func (f *StorageFactory) SetStorage(storage Storage) { +func (f *Factory) SetStorage(storage Storage) { f.mu.Lock() defer f.mu.Unlock() f.storage = storage