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

Abierto
  • ¿Algo no está bien?
#7,096 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

@TonyWu2333 ya está trabajando en esto.

Desde el 5/10/2026.

Evaluación

Dificultad
3/5
Tiempo estimado
1-2 días
Aptitud para principiantes
76/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Tranquilo
Stack tecnológico
go
Área
backend, distributed-systems, testing-qa

Línea de trabajo

Comienza con pkg/app/pipedv1/metadatastore/registry.go; después, inspecciona el bloqueo existente en store.go y las pruebas de registry_test.go. Ejecuta go test -race ./pkg/app/pipedv1/metadatastore/... y usa el escenario de acceso concurrente proporcionado para verificar que el registro siga libre de condiciones de carrera y no entre en pánico mientras se registran, acceden y eliminan deployments.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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
Lenguaje dominante
Go
Estrellas
1.4k
Forks
391
Merge medio
4 d 1 h
PR fusionados (30 d)
58

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de pipe-cd/pipecd

Todos los issues de pipe-cd/pipecd

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.