deduplicate registration-time duplicates from match directives,

caddy/app.go

@@ -126,19 +126,49 @@ func (f *FrankenPHPApp) addModuleWorkers(workers ...workerConfig) ([]workerConfi
 		if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(w.FileName) {
 			w.FileName = filepath.Join(frankenphp.EmbeddedAppPath, w.FileName)
 		}
+	}
+
+	// A php_server directive is provisioned once per route it's embedded in. Only the first embed
+	// registers its pools; later embeds reuse them by position, never touching other directives (#2477).
+	var registered []workerConfig
+	if len(workers) > 0 && workers[0].routeGroup != "" {
+		registered = f.moduleWorkersInRouteGroup(workers[0].routeGroup)
+	}
 
-		if w.Name == "" {
-			w.Name = f.generateUniqueModuleWorkerName(w.FileName)
-		} else if !strings.HasPrefix(w.Name, "m#") {
-			w.Name = "m#" + w.Name
+	for i := range workers {
+		if i < len(registered) {
+			workers[i].Name = registered[i].Name
+			continue
 		}
 
-		f.Workers = append(f.Workers, *w)
+		f.registerModuleWorker(&workers[i])
 	}
 
 	return workers, nil
 }
 
+func (f *FrankenPHPApp) registerModuleWorker(w *workerConfig) {
+	if w.Name == "" {
+		w.Name = f.generateUniqueModuleWorkerName(w.FileName)
+	} else if !strings.HasPrefix(w.Name, "m#") {
+		w.Name = "m#" + w.Name
+	}
+
+	f.Workers = append(f.Workers, *w)
+}
+
+// moduleWorkersInRouteGroup returns the registered workers of one directive, in registration order.
+func (f *FrankenPHPApp) moduleWorkersInRouteGroup(routeGroup string) []workerConfig {
+	var group []workerConfig
+	for _, w := range f.Workers {
+		if w.routeGroup == routeGroup {
+			group = append(group, w)
+		}
+	}
+
+	return group
+}
+
 func (f *FrankenPHPApp) Start() error {
 	repl := caddy.NewReplacer()
 

caddy/caddy_test.go

@@ -858,6 +858,69 @@ func TestWorkerMetrics(t *testing.T) {
 		))
 }
 
+// #2477: verify one pool per worker, no "_0" duplicates.
+func TestPhpServerWorkerMatchPoolCount(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	initServer(t, tester, `
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port `+testPort+`
+		https_port 9443
+		metrics
+	}
+
+	localhost:`+testPort+` {
+		php_server {
+			root ../testdata
+			worker {
+				file ../testdata/dedup-match-worker.php
+				num 1
+				match /match/*
+			}
+			worker {
+				file ../testdata/dedup-plain-worker.php
+				num 1
+			}
+		}
+	}
+	`, "caddyfile")
+
+	matchedWorker, _ := fastabs.FastAbs("../testdata/dedup-match-worker.php")
+	plainWorker, _ := fastabs.FastAbs("../testdata/dedup-plain-worker.php")
+
+	// the matched (non-.php) path must still be served by its worker
+	tester.AssertGetResponse("http://localhost:"+testPort+"/match/anything", http.StatusOK, "dedup-match-worker")
+
+	resp, err := http.Get("http://localhost:2999/metrics")
+	require.NoError(t, err, "failed to fetch metrics")
+	t.Cleanup(func() {
+		require.NoError(t, resp.Body.Close())
+	})
+
+	metrics := new(bytes.Buffer)
+	_, err = metrics.ReadFrom(resp.Body)
+	require.NoError(t, err, "failed to read metrics")
+
+	var pools []string
+	for _, line := range strings.Split(metrics.String(), "\n") {
+		if !strings.HasPrefix(line, "frankenphp_total_workers{worker=") {
+			continue
+		}
+		if !strings.Contains(line, "dedup-match-worker.php") && !strings.Contains(line, "dedup-plain-worker.php") {
+			continue
+		}
+		pools = append(pools, line)
+	}
+
+	require.Len(t, pools, 2, "expected exactly one pool per distinct worker, got: %v", pools)
+	joined := strings.Join(pools, "\n")
+	require.NotContains(t, joined, escapeMetricLabel(matchedWorker)+`_0`, "matched worker must not be registered twice: %v", pools)
+	require.NotContains(t, joined, escapeMetricLabel(plainWorker)+`_0`, "plain worker must not be registered twice: %v", pools)
+	require.Contains(t, joined, escapeMetricLabel(matchedWorker), "matched worker pool must be present: %v", pools)
+	require.Contains(t, joined, escapeMetricLabel(plainWorker), "plain worker pool must be present: %v", pools)
+}
+
 func TestNamedWorkerMetrics(t *testing.T) {
 	var wg sync.WaitGroup
 	tester := caddytest.NewTester(t)

caddy/config_test.go

@@ -1,12 +1,104 @@
 package caddy
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/stretchr/testify/require"
 )
 
+func TestPhpServerWorkerMatchNoDuplicatePools(t *testing.T) {
+	const config = `
+	{
+		php {
+			worker {
+				file ../testdata/worker-with-env.php
+				num 2
+				match /index.php/*
+			}
+			worker {
+				file ../testdata/worker-with-counter.php
+				num 1
+			}
+		}
+	}`
+
+	// a fresh copy per call mirrors the two module instances Caddy provisions for one directive
+	parseWorkers := func(routeGroup string) []workerConfig {
+		d := caddyfile.NewTestDispenser(config)
+		module := &FrankenPHPModule{}
+		require.NoError(t, module.UnmarshalCaddyfile(d))
+		require.Len(t, module.Workers, 2, "block should parse to two workers")
+		for i := range module.Workers {
+			module.Workers[i].routeGroup = routeGroup
+		}
+
+		return module.Workers
+	}
+
+	app := &FrankenPHPApp{}
+
+	first, err := app.addModuleWorkers(parseWorkers("g1")...)
+	require.NoError(t, err)
+	second, err := app.addModuleWorkers(parseWorkers("g1")...)
+	require.NoError(t, err)
+
+	require.Len(t, app.Workers, 2, "each worker must be registered exactly once")
+
+	for _, w := range app.Workers {
+		require.False(t, strings.HasSuffix(w.Name, "_0"),
+			"no _0-suffixed duplicate pool may exist, got %q", w.Name)
+	}
+
+	// both embeds must resolve to the same pools for serve-time matching
+	require.Len(t, first, 2)
+	require.Len(t, second, 2)
+	for i := range first {
+		require.Equal(t, first[i].Name, second[i].Name,
+			"both routes must reference the same pool name for worker %d", i)
+	}
+}
+
+func TestPhpServerSeparateDirectivesKeepDistinctPools(t *testing.T) {
+	worker := func(routeGroup string) workerConfig {
+		return workerConfig{FileName: "../testdata/worker-with-env.php", Num: 2, routeGroup: routeGroup}
+	}
+
+	app := &FrankenPHPApp{}
+	site1, err := app.addModuleWorkers(worker("g1"))
+	require.NoError(t, err)
+	site2, err := app.addModuleWorkers(worker("g2"))
+	require.NoError(t, err)
+
+	require.Len(t, app.Workers, 2, "identical workers from separate directives must stay separate pools")
+	require.NotEqual(t, site1[0].Name, site2[0].Name, "separate directives must not share a pool name")
+	require.True(t, strings.HasSuffix(site2[0].Name, "_0"),
+		"the second directive's pool must take a unique _0 name, got %q", site2[0].Name)
+}
+
+func TestPhpServerEmbedReuseIsPositional(t *testing.T) {
+	embed := func() []workerConfig {
+		return []workerConfig{
+			{FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"},
+			{FileName: "../testdata/worker-with-env.php", Num: 1, MatchPath: []string{"/x/*"}, routeGroup: "g1"},
+		}
+	}
+
+	app := &FrankenPHPApp{}
+	first, err := app.addModuleWorkers(embed()...)
+	require.NoError(t, err)
+	second, err := app.addModuleWorkers(embed()...)
+	require.NoError(t, err)
+
+	require.Len(t, app.Workers, 2, "two identical workers in one block stay two pools, just as without the duplicate embed")
+	require.NotEqual(t, first[0].Name, first[1].Name, "the second identical worker must take its own _0 pool")
+	require.True(t, strings.HasSuffix(first[1].Name, "_0"), "got %q", first[1].Name)
+	for i := range first {
+		require.Equal(t, first[i].Name, second[i].Name, "the duplicate embed must reuse pools by position for worker %d", i)
+	}
+}
+
 func TestModuleWorkerDuplicateFilenamesFail(t *testing.T) {
 	// Create a test configuration with duplicate worker filenames
 	configWithDuplicateFilenames := `

caddy/module.go

@@ -45,6 +45,8 @@ type FrankenPHPModule struct {
 	Env map[string]string `json:"env,omitempty"`
 	// Workers configures the worker scripts to start.
 	Workers []workerConfig `json:"workers,omitempty"`
+	// RouteGroup is set automatically to pair the route embeds of one php_server directive (#2477). Do not set it manually.
+	RouteGroup string `json:"route_group,omitempty"`
 
 	resolvedDocumentRoot        string
 	preparedEnv                 frankenphp.PreparedEnv
@@ -92,6 +94,7 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
 		}
 
 		wc.requestOptions = append(wc.requestOptions, loggerOpt)
+		wc.routeGroup = f.RouteGroup
 		f.Workers[i] = wc
 	}
 
@@ -339,6 +342,8 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error)
 	return m, err
 }
 
+const routeGroupStateKey = "frankenphp.worker_route_group_seq"
+
 // parsePhpServer parses the php_server directive, which has a similar syntax
 // to the php_fastcgi directive. A line such as this:
 //
@@ -371,6 +376,12 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
 		return nil, h.ArgErr()
 	}
 
+	// per-adaptation counter: identical for both embeds of this directive, distinct for every other
+	// (including separate snippet imports), and stable across re-adaptation since State resets each time
+	seq, _ := h.State[routeGroupStateKey].(int)
+	h.State[routeGroupStateKey] = seq + 1
+	routeGroup := strconv.Itoa(seq)
+
 	// set up FrankenPHP
 	phpsrv := FrankenPHPModule{}
 
@@ -481,6 +492,8 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
 		}
 	}
 
+	phpsrv.RouteGroup = routeGroup
+
 	// set up a route list that we'll append to
 	routes := caddyhttp.RouteList{}
 

caddy/workerconfig.go

@@ -46,6 +46,7 @@ type workerConfig struct {
 	requestOptions []frankenphp.RequestOption
 	absFileName    string
 	matchRelPath   string // pre-computed relative URL path for fast matching
+	routeGroup     string // identifies the php_server directive whose route embeds share this pool
 }
 
 func unmarshalWorker(d *caddyfile.Dispenser) (workerConfig, error) {

testdata/dedup-match-worker.php

@@ -0,0 +1,8 @@
+<?php
+
+// Dedicated worker for TestPhpServerWorkerMatchPoolCount (#2477 regression guard).
+// Used by no other test so its metric label is hermetic.
+while (frankenphp_handle_request(static function (): void {
+    echo 'dedup-match-worker';
+})) {
+}

testdata/dedup-plain-worker.php

@@ -0,0 +1,8 @@
+<?php
+
+// Dedicated worker for TestPhpServerWorkerMatchPoolCount (#2477 regression guard).
+// Used by no other test so its metric label is hermetic.
+while (frankenphp_handle_request(static function (): void {
+    echo 'dedup-plain-worker';
+})) {
+}