Azure / azure-storage-blob-go

Microsoft Azure Blob Storage Library for Go
MIT License
157 stars 102 forks source link

Goroutine Leak with client.DownloadBuffer method #335

Closed iyunbo closed 3 months ago

iyunbo commented 1 year ago

Which version of the SDK was used?

github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v0.6.1

Which platform are you using? (ex: Windows, Linux, Debian)

MacOs, Linux

What problem was encountered?

Goroutine leak when calling DownloadBuffer with context.Background() this could be linked to this issue: https://github.com/Azure/azure-storage-blob-go/issues/180

How can we reproduce the problem in the simplest way?

the following code allows the reproduction of the goroutine leak, don't forget setting valid values for container, key and storageAccount:

package test

import (
    "context"
    "fmt"
    "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
    "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
    "runtime"
    "testing"

    "github.com/rs/zerolog/log"
)

func TestDownloadGoroutineLeak(t *testing.T) {
    const iterations = 5
    const container = "myContainer"
    const key = "myKey"
    const storageAccount = "myStorageAccount"

    credential, err := azidentity.NewDefaultAzureCredential(nil)
    if err != nil {
        t.Fatalf("failed setting up Credential: %v", err)
    }
    client, err := azblob.NewClient(fmt.Sprintf("https://%s.blob.core.windows.net", storageAccount), credential, nil)
    if err != nil {
        t.Fatalf("failed creating Azure Blob client: %v", err)
    }
    for i := 0; i < iterations; i++ {
        ctx := context.Background()
        buf := make([]byte, 1024*1024*1024)
        size, err := Download(ctx, *client, container, key, buf)
        if err != nil {
            log.Error().Msgf("could not download file: %v", err)
        } else {
            log.Info().Msgf("downloaded %d bytes", size)
        }
        log.Info().Msgf("number of goroutines: %d", runtime.NumGoroutine())
    }

}

func Download(ctx context.Context, client azblob.Client, container, key string, buf []byte) (int64, error) {
    var size int64
    var err error

    size, err = client.DownloadBuffer(ctx, container, key, buf, &azblob.DownloadBufferOptions{
        BlockSize:   12 * 1204 * 1024,
        Concurrency: 1,
    })
    if err != nil {
        return 0, fmt.Errorf("azure Blobstore: %w", err)
    }

    log.Info().Msgf("download %s/%s", container, key)

    return size, nil
}

Have you found a mitigation/solution?

Yes. Instead of passing a context.Background(), use a context.WithCancel() or context.WithTimeout() can avoid the leak. for example:

for i := 0; i < iterations; i++ {
        ctx, downloadComplete := context.WithTimeout(context.Background(), 30*time.Second)
        buf := make([]byte, 1024*1024*1024)
        size, err := Download(ctx, *client, container, key, buf)
        if err != nil {
            log.Error().Msgf("could not download file: %v", err)
        } else {
            log.Info().Msgf("downloaded %d bytes", size)
        }
        downloadComplete()
        log.Info().Msgf("number of goroutines: %d", runtime.NumGoroutine())
}
iyunbo commented 3 months ago

this issue is fixed by (or an earlier version) - github.com/Azure/azure-sdk-for-go/sdk/storage/azblob v1.3.2