[pipedv1] Data race in MetadataStoreRegistry causes fatal panic under concurrent deployments

Đang mở
  • Có gì không ổn?
#7,096 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 2 ngày

@TonyWu2333 đang làm issue này rồi.

Từ ngày 5/10/2026.

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
76/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Ít trao đổi
Công nghệ
go
Lĩnh vực
backend, distributed-systems, testing-qa

Hướng nghiên cứu

Bắt đầu với pkg/app/pipedv1/metadatastore/registry.go, sau đó kiểm tra cơ chế locking hiện có trong store.go và các test trong registry_test.go. Chạy go test -race ./pkg/app/pipedv1/metadatastore/... và sử dụng kịch bản truy cập đồng thời được cung cấp để xác minh rằng registry vẫn không có race và không panic trong khi các deployment được đăng ký, truy cập và xóa.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

kind/bug

Description:

What happened:

MetadataStoreRegistry in pkg/app/pipedv1/metadatastore/registry.go holds a stores map[string]*metadataStore field with no synchronization primitive, causing a fatal concurrent map read and map write panic when multiple deployments run simultaneously.

The inner metadataStore correctly protects its own metadata maps with sync.RWMutex (store.go:47-49), but the outer registry map is unprotected. Two independent goroutine families access it without coordination:

  1. Writers — Register(d) (registry.go:44) and Delete(d.Id) (registry.go:51) are called from planner/scheduler goroutines as deployments start and finish.
  2. Readers — All 7 gRPC handler methods (GetStageMetadata, PutStageMetadata, PutStageMetadataMulti, GetDeploymentPluginMetadata, PutDeploymentPluginMetadata, PutDeploymentPluginMetadataMulti, GetDeploymentSharedMetadata) read r.stores[req.DeploymentId] (registry.go:47-119) from piped's plugin-service gRPC server goroutines.

When two or more applications deploy concurrently, a gRPC handler reading the map for deployment A races with Register/Delete for deployment B → Go runtime panics with fatal error: concurrent map read and map write → entire piped process aborts, killing all active deployments on that agent.

What you expected to happen:

MetadataStoreRegistry should safely serialize concurrent access to the stores map. Multiple deployments should coexist without data races or crashes.

How to reproduce it:

Add the following test to pkg/app/pipedv1/metadatastore/registry_test.go and run with the Go race detector:

package metadatastore

import (
	"context"
	"fmt"
	"sync"
	"testing"

	"github.com/pipe-cd/pipecd/pkg/model"
	service "github.com/pipe-cd/pipecd/pkg/plugin/pipedservice"
)

func TestRegistryConcurrentAccess(t *testing.T) {
	t.Parallel()
	ac := &fakeAPIClient{
		shared:  make(map[string]string),
		plugins: make(map[string]metadata),
		stages:  make(map[string]metadata),
	}
	r := NewMetadataStoreRegistry(ac)
	ctx := context.Background()

	var wg sync.WaitGroup
	const deployments = 20
	const opsPerDeployment = 8

	for i := 0; i < deployments; i++ {
		// Writer: Register + Delete
		wg.Add(1)
		go func(idx int) {
			defer wg.Done()
			id := fmt.Sprintf("deploy-%d", idx)
			d := &model.Deployment{
				Id: id,
				MetadataV2: &model.DeploymentMetadata{
					Shared:  &model.DeploymentMetadata_KeyValues{KeyValues: map[string]string{}},
					Plugins: map[string]*model.DeploymentMetadata_KeyValues{},
				},
				Stages: []*model.PipelineStage{{Id: "stage-1"}},
			}
			r.Register(d)
			r.Delete(id)
		}(i)

		// Readers: simulate gRPC handler calls
		for j := 0; j < opsPerDeployment; j++ {
			wg.Add(1)
			go func(idx int) {
				defer wg.Done()
				id := fmt.Sprintf("deploy-%d", idx)
				_, _ = r.PutStageMetadata(ctx, &service.PutStageMetadataRequest{
					DeploymentId: id, StageId: "stage-1", Key: "k", Value: "v",
				})
			}(i)
		}
	}
	wg.Wait()
}

Run command: go test -race ./pkg/app/pipedv1/metadatastore/...

Expected output (on unpatched master):

==================
WARNING: DATA RACE
Read at 0x... by goroutine ...:
    (*MetadataStoreRegistry).PutStageMetadata
        .../registry.go:59
Previous write at 0x... by goroutine ...:
    (*MetadataStoreRegistry).Register
        .../registry.go:44
==================
fatal error: concurrent map read and map write

Root Cause & Suggested Fix:

The MetadataStoreRegistry struct (registry.go:26-32) contains only apiClient and stores — no mutex. Register/Delete modify the map from lifecycle goroutines, while all 7 gRPC handlers read it from server goroutines, with no synchronization between them.

To fix, add a sync.RWMutex to the struct:

  1. RLock in all 7 gRPC handlers for the map lookup (snapshot the pointer, release lock, then delegate to inner store).
  2. Lock in Register and Delete for map mutations.
  3. Add a dedicated race test using channel-barrier synchronization for deterministic reproduction.

This pattern mirrors the existing locking convention one level down (metadataStore uses sync.RWMutex for sharedMu/pluginsMu/stagesMu) and introduces zero deadlock risk (registry lock is released before any inner store method or gRPC call).

Environment:

  • piped version: master (commit 56fb80e)
  • control-plane version: master
  • Others: Go 1.26.2, go test -race
Ngôn ngữ chính
Go
Star
1.4k
Fork
391
Merge trung bình
4 ngày 1 giờ
Pull request đã merge (30 ngày)
58

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của pipe-cd/pipecd

Tất cả issue của pipe-cd/pipecd

Issue tương tự

Thêm issue về Go

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.