diff --git a/pkg/storage/chunked.go b/pkg/storage/chunked.go index be411e66..11c0827a 100644 --- a/pkg/storage/chunked.go +++ b/pkg/storage/chunked.go @@ -57,6 +57,10 @@ type chunkedSecrets struct { hashMu sync.Mutex hash hash.Hash64 hashEncoding *base32.Encoding + + // chunkWriters tracks the goroutines started by decodeRelease that feed + // chunk data into the decoder, so tests can check that they all exit. + chunkWriters sync.WaitGroup } func (c *chunkedSecrets) Create(key string, rls *release.Release) error { @@ -398,7 +402,9 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1. } pr, pw := io.Pipe() + c.chunkWriters.Add(1) go func() { + defer c.chunkWriters.Done() defer pw.Close() firstChunkData, ok := indexSecret.Data["chunk"] if !ok { @@ -427,6 +433,9 @@ func (c *chunkedSecrets) decodeRelease(ctx context.Context, indexSecret *corev1. } }() + // Close the read side when decoding returns so that the writer goroutine + // does not block forever if decoding stops before all chunk data is read. + defer pr.Close() gzr, err := gzip.NewReader(pr) if err != nil { return nil, fmt.Errorf("failed to create gzip reader: %w", err) diff --git a/pkg/storage/chunked_test.go b/pkg/storage/chunked_test.go index d8bd0c12..9112baf2 100644 --- a/pkg/storage/chunked_test.go +++ b/pkg/storage/chunked_test.go @@ -95,6 +95,34 @@ var _ = Describe("chunkedSecrets", func() { _, err := maxReadDriver.Get(releaseKey(rel)) Expect(err).To(MatchError(ContainSubstring("release too large"))) }) + It("should not leak goroutines if the release fails to decode", func() { + corruptIndex := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "corrupt-release.v1"}, + Type: SecretTypeChunkedIndex, + Data: map[string][]byte{ + "extraChunks": []byte("[]"), + // Larger than the gzip reader's read buffer, so the + // reader stops before consuming all of the chunk data. + "chunk": make([]byte, 10*chunkSize), + }, + } + _, err := secretInterface.Create(context.Background(), corruptIndex, metav1.CreateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + for range 5 { + _, err := chunkedDriver.Get(corruptIndex.Name) + Expect(err).To(MatchError(ContainSubstring("failed to create gzip reader"))) + } + + // Wait only for the chunk writer goroutines started by this + // driver, so unrelated goroutines cannot affect the result. + writersDone := make(chan struct{}) + go func() { + chunkedDriver.(*chunkedSecrets).chunkWriters.Wait() + close(writersDone) + }() + Eventually(writersDone).Should(BeClosed()) + }) }) var _ = Describe("Update", func() {