diff --git a/.github/actions/setup-php/action.yaml b/.github/actions/setup-php/action.yaml index 9bc8c49208..6227376e84 100644 --- a/.github/actions/setup-php/action.yaml +++ b/.github/actions/setup-php/action.yaml @@ -12,6 +12,8 @@ runs: with: php-version: ${{ inputs.php-version }} ini-file: development + # JIT's fixed-address mappings can race Go thread creation during startup. + ini-values: opcache.jit=disable coverage: none tools: none env: diff --git a/caddy/admin_test.go b/caddy/admin_test.go index b5fe37daae..d44deaa8d6 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -21,7 +21,7 @@ import ( func TestRestartWorkerViaAdminApi(t *testing.T) { tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -54,7 +54,7 @@ func TestRestartWorkerViaAdminApi(t *testing.T) { func TestShowTheCorrectThreadDebugStatus(t *testing.T) { tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -77,6 +77,12 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) { } `, "caddyfile") + // Regular threads finish their startup transition asynchronously. + require.Eventually(t, func() bool { + threads := frankenphp.DebugState().ThreadDebugStates + return len(threads) == 3 && threads[0].State == "ready" + }, 5*time.Second, 10*time.Millisecond, "regular PHP thread did not become ready") + debugState := getDebugState(t, tester) // assert that the correct threads are present in the thread info @@ -89,7 +95,7 @@ func TestShowTheCorrectThreadDebugStatus(t *testing.T) { func TestThreadDebugStateMetricsAfterRequests(t *testing.T) { tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -132,7 +138,7 @@ func TestAutoScaleWorkerThreads(t *testing.T) { maxTries := 10 requestsPerTry := 200 tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -189,7 +195,7 @@ func TestAutoScaleRegularThreadsOnAutomaticThreadLimit(t *testing.T) { maxTries := 10 requestsPerTry := 200 tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -278,7 +284,7 @@ func getNumThreads(t *testing.T, tester *caddytest.Tester) int { func TestAddModuleWorkerViaAdminApi(t *testing.T) { // Initialize a server with admin API enabled tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 331b13ee5d..6f1c52f4ed 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -23,12 +23,11 @@ import ( ) // initServer initializes a Caddy test server and waits for it to be ready. -// After InitServer, it polls the server to handle a race condition on macOS where -// SO_REUSEPORT can briefly route connections to the old listener being shut down, -// resulting in "connection reset by peer". +// It polls the HTTP endpoint after loading the configuration to wait for the +// listener to start accepting requests. func initServer(t *testing.T, tester *caddytest.Tester, config string, format string) { t.Helper() - tester.InitServer(config, format) + initTestServer(t, tester, config, format) client := &http.Client{Timeout: 1 * time.Second} require.Eventually(t, func() bool { @@ -65,6 +64,9 @@ func escapeMetricLabel(s string) string { } func TestMain(m *testing.M) { + // Bound helpers using http.Get, including diagnostics and metrics requests. + http.DefaultClient.Timeout = caddytest.Default.TestRequestTimeout + // setup custom environment vars for TestOsEnv if os.Setenv("ENV1", "value1") != nil || os.Setenv("ENV2", "value2") != nil { fmt.Println("Failed to set environment variables for tests") @@ -98,8 +100,8 @@ func TestPHP(t *testing.T) { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -161,8 +163,8 @@ func TestWorker(t *testing.T) { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -213,9 +215,9 @@ func TestGlobalAndModuleWorker(t *testing.T) { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=module") tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=global") - wg.Done() }(i) } wg.Wait() @@ -287,9 +289,9 @@ func TestNamedModuleWorkers(t *testing.T) { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=one") tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=two") - wg.Done() }(i) } wg.Wait() @@ -722,8 +724,8 @@ func TestMetrics(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -798,8 +800,8 @@ func TestWorkerMetrics(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -955,8 +957,8 @@ func TestNamedWorkerMetrics(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -1049,8 +1051,8 @@ func TestAutoWorkerConfig(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -1228,7 +1230,7 @@ func TestOsEnv(t *testing.T) { require.NoError(t, listener.Close()) tester := caddytest.NewTester(t) - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 @@ -1285,11 +1287,11 @@ func TestMaxWaitTime(t *testing.T) { wg.Add(10) for range 10 { go func() { + defer wg.Done() statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10", t) if statusCode == http.StatusServiceUnavailable { success.Store(true) } - wg.Done() }() } wg.Wait() @@ -1332,11 +1334,11 @@ func TestMaxWaitTimeWorker(t *testing.T) { wg.Add(10) for range 10 { go func() { + defer wg.Done() statusCode := getStatusCode("http://localhost:"+testPort+"/sleep.php?sleep=10&iteration=1", t) if statusCode == http.StatusServiceUnavailable { success.Store(true) } - wg.Done() }() } wg.Wait() @@ -1425,8 +1427,8 @@ func TestMultiWorkersMetrics(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -1533,8 +1535,8 @@ func TestDisabledMetrics(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/index.php?i=%d", i), http.StatusOK, fmt.Sprintf("I am by birth a Genevese (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -1642,8 +1644,8 @@ func TestWorkerRestart(t *testing.T) { for i := range 10 { wg.Add(1) go func(i int) { + defer wg.Done() tester.AssertGetResponse(fmt.Sprintf("http://localhost:"+testPort+"/worker-restart.php?i=%d", i), http.StatusOK, fmt.Sprintf("Counter (%d)", i)) - wg.Done() }(i) } wg.Wait() @@ -1806,7 +1808,7 @@ func TestDd(t *testing.T) { func TestOpcacheReset(t *testing.T) { tester := caddytest.NewTester(t) tester.Client.Timeout = 60 * time.Second - tester.InitServer(` + initTestServer(t, tester, ` { skip_install_trust admin localhost:2999 diff --git a/caddy/hotreload_test.go b/caddy/hotreload_test.go index 9eed620453..c8f802fa32 100644 --- a/caddy/hotreload_test.go +++ b/caddy/hotreload_test.go @@ -9,7 +9,6 @@ import ( "os" "path/filepath" "strings" - "sync" "testing" "time" @@ -30,7 +29,7 @@ func TestHotReload(t *testing.T) { // SSE roundtrip below on slow CI runners (notably emulated armv7). // 30s keeps the test bounded so a real regression fails fast. tester.Client.Timeout = 30 * time.Second - tester.InitServer(` + initTestServer(t, tester, ` { debug skip_install_trust @@ -53,49 +52,56 @@ func TestHotReload(t *testing.T) { } `, "caddyfile") - var connected, received sync.WaitGroup - - connected.Add(1) - received.Go(func() { - cx, cancel := context.WithCancel(t.Context()) - req, _ := http.NewRequest(http.MethodGet, "http://localhost:"+testPort+u, nil) - req = req.WithContext(cx) - resp := tester.AssertResponseCode(req, http.StatusOK) - + cx, cancel := context.WithCancel(t.Context()) + defer cancel() + req, err := http.NewRequestWithContext(cx, http.MethodGet, "http://localhost:"+testPort+u, nil) + require.NoError(t, err) + resp := tester.AssertResponseCode(req, http.StatusOK) + defer func() { require.NoError(t, resp.Body.Close()) }() + + // Wait for the first bytes before changing the file, so the subscription + // is ready. A failed request or read must not leave a readiness wait blocked. + buf := make([]byte, 1024) + _, err = resp.Body.Read(buf) + require.NoError(t, err) + + // Return read failures to the test goroutine. Buffer the result so this + // goroutine can exit even if a failed file write ends the test first. + received := make(chan error, 1) + go func() { var receivedBody strings.Builder - - buf := make([]byte, 1024) - isConnected := false for { n, err := resp.Body.Read(buf) - if n > 0 { - receivedBody.Write(buf[:n]) - } - if !isConnected { - // wait for the first bytes before marking the client as connected - isConnected = true - connected.Done() - } + receivedBody.Write(buf[:n]) if strings.Contains(receivedBody.String(), "index.php") { - cancel() - - break + received <- nil + return } - // Surface the read error only after checking the buffer: on - // Windows the SSE server sometimes flushes the event and closes - // the connection in the same syscall, so Read returns (n>0, EOF) - // and we'd otherwise fail despite having the data we wanted. + // A read may return both the expected event and an error. + if err != nil { + received <- err + return + } + } + }() + + // The native watcher starts asynchronously, independently of the SSE + // subscription. Retry changes until one is observed, leaving more time + // between writes than the watcher's 150 ms debounce interval. + ticker := time.NewTicker(250 * time.Millisecond) + defer ticker.Stop() +waitForReload: + for { + require.NoError(t, os.WriteFile(indexFile, []byte("