fix: timeouts handling on macOS (#1435)

* ci: run tests on macOS

* debug

* debug

* fix

* nobrotli

* install brotli

* fix

* fix

* Also registers php.ini if ZEND_MAX_EXECUTION_TIMERS is disabled.

* Removes max_execution_time from tests (it gets overwritten on mac)

* tiny refacto

* fix free

* cs

---------

Co-authored-by: Alliballibaba <[email protected]>

Author: dunglas

.github/workflows/tests.yaml

@@ -14,7 +14,8 @@ on:
 permissions:
   contents: read
 jobs:
-  tests:
+  tests-linux:
+    name: Tests (Linux, PHP ${{ matrix.php-versions }})
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -71,3 +72,39 @@ jobs:
         if: matrix.php-versions == '8.4'
         with:
           version: latest
+  tests-mac:
+    name: Tests (macOS, PHP 8.4)
+    runs-on: macos-latest
+    env:
+      GOEXPERIMENT: cgocheck2
+      HOMEBREW_NO_AUTO_UPDATE: 1
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: "1.24"
+          cache-dependency-path: |
+            go.sum 
+            caddy/go.sum
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.4
+          ini-file: development
+          coverage: none
+          tools: none
+        env:
+          phpts: ts
+          debug: true
+      - name: Set Set CGO flags
+        run: |
+          {
+           echo "CGO_CFLAGS=-I/opt/homebrew/include/ $(php-config --includes)"
+           echo "CGO_LDFLAGS=-L/opt/homebrew/lib/ $(php-config --ldflags) $(php-config --libs)"
+          } >> "${GITHUB_ENV}"
+      - name: Build
+        run: go build -tags nowatcher
+      - name: Run library tests
+        run: go test -tags nowatcher -race -v ./...
+      - name: Run Caddy module tests
+        working-directory: caddy/
+        run: go test -tags nowatcher,nobadger,nomysql,nopgx -race -v ./...

CONTRIBUTING.md

@@ -149,16 +149,13 @@ docker buildx bake -f docker-bake.hcl --pull --no-cache --push
 3. Enable `tmate` to connect to the container
 
     ```patch
-        -
-          name: Set CGO flags
+        - name: Set CGO flags
           run: echo "CGO_CFLAGS=$(php-config --includes)" >> "$GITHUB_ENV"
-    +   -
-    +     run: |
+    +   - run: |
     +       sudo apt install gdb
     +       mkdir -p /home/runner/.config/gdb/
     +       printf "set auto-load safe-path /\nhandle SIG34 nostop noprint pass" > /home/runner/.config/gdb/gdbinit
-    +   -
-    +     uses: mxschmitt/action-tmate@v3
+    +   - uses: mxschmitt/action-tmate@v3
     ```
 
 4. Connect to the container

caddy/caddy_test.go

@@ -660,7 +660,7 @@ func TestPHPIniConfiguration(t *testing.T) {
 			frankenphp {
 				num_threads 2
 				worker ../testdata/ini.php 1
-				php_ini max_execution_time 100
+				php_ini upload_max_filesize 100M
 				php_ini memory_limit 10000000
 			}
 		}
@@ -673,7 +673,7 @@ func TestPHPIniConfiguration(t *testing.T) {
 		}
 		`, "caddyfile")
 
-	testSingleIniConfiguration(tester, "max_execution_time", "100")
+	testSingleIniConfiguration(tester, "upload_max_filesize", "100M")
 	testSingleIniConfiguration(tester, "memory_limit", "10000000")
 }
 
@@ -688,7 +688,7 @@ func TestPHPIniBlockConfiguration(t *testing.T) {
 			frankenphp {
 				num_threads 1
 				php_ini {
-					max_execution_time 15
+					upload_max_filesize 100M
 					memory_limit 20000000
 				}
 			}
@@ -702,7 +702,7 @@ func TestPHPIniBlockConfiguration(t *testing.T) {
 		}
 		`, "caddyfile")
 
-	testSingleIniConfiguration(tester, "max_execution_time", "15")
+	testSingleIniConfiguration(tester, "upload_max_filesize", "100M")
 	testSingleIniConfiguration(tester, "memory_limit", "20000000")
 }
 

frankenphp.c

@@ -33,13 +33,6 @@
 ZEND_TSRMLS_CACHE_DEFINE()
 #endif
 
-/* Timeouts are currently fundamentally broken with ZTS except on Linux and
- * FreeBSD: https://bugs.php.net/bug.php?id=79464 */
-#ifndef ZEND_MAX_EXECUTION_TIMERS
-static const char HARDCODED_INI[] = "max_execution_time=0\n"
-                                    "max_input_time=-1\n\0";
-#endif
-
 static const char *MODULES_TO_RELOAD[] = {"filter", "session", NULL};
 
 frankenphp_version frankenphp_get_version() {
@@ -900,25 +893,18 @@ static void *php_main(void *arg) {
 
   sapi_startup(&frankenphp_sapi_module);
 
-#ifndef ZEND_MAX_EXECUTION_TIMERS
-#if (PHP_VERSION_ID >= 80300)
-  frankenphp_sapi_module.ini_entries = HARDCODED_INI;
+#ifdef ZEND_MAX_EXECUTION_TIMERS
+  /* overwrite php.ini with custom user settings */
+  char *php_ini_overrides = go_get_custom_php_ini(false);
 #else
-  frankenphp_sapi_module.ini_entries = malloc(sizeof(HARDCODED_INI));
-  if (frankenphp_sapi_module.ini_entries == NULL) {
-    perror("malloc failed");
-    exit(EXIT_FAILURE);
-  }
-  memcpy(frankenphp_sapi_module.ini_entries, HARDCODED_INI,
-         sizeof(HARDCODED_INI));
+  /* overwrite php.ini with custom user settings and disable
+   * max_execution_timers */
+  char *php_ini_overrides = go_get_custom_php_ini(true);
 #endif
-#else
-  /* overwrite php.ini with custom user settings */
-  char *php_ini_overrides = go_get_custom_php_ini();
+
   if (php_ini_overrides != NULL) {
     frankenphp_sapi_module.ini_entries = php_ini_overrides;
   }
-#endif
 
   frankenphp_sapi_module.startup(&frankenphp_sapi_module);
 
@@ -938,13 +924,13 @@ static void *php_main(void *arg) {
   tsrm_shutdown();
 #endif
 
-#if (PHP_VERSION_ID < 80300)
   if (frankenphp_sapi_module.ini_entries) {
-    free(frankenphp_sapi_module.ini_entries);
+    free((char *)frankenphp_sapi_module.ini_entries);
     frankenphp_sapi_module.ini_entries = NULL;
   }
-#endif
+
   go_frankenphp_shutdown_main_thread();
+
   return NULL;
 }
 

phpmainthread.go

@@ -8,7 +8,7 @@ package frankenphp
 // #include "frankenphp.h"
 import "C"
 import (
-	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/dunglas/frankenphp/internal/memory"
@@ -191,16 +191,28 @@ func go_frankenphp_shutdown_main_thread() {
 }
 
 //export go_get_custom_php_ini
-func go_get_custom_php_ini() *C.char {
+func go_get_custom_php_ini(disableTimeouts C.bool) *C.char {
 	if mainThread.phpIni == nil {
-		return nil
+		mainThread.phpIni = make(map[string]string)
 	}
 
-	// pass the php.ini overrides to PHP before startup
+	// Timeouts are currently fundamentally broken
+	// with ZTS except on Linux and FreeBSD: https://bugs.php.net/bug.php?id=79464
+	// Disable timeouts if ZEND_MAX_EXECUTION_TIMERS is not supported
+	if disableTimeouts {
+		mainThread.phpIni["max_execution_time"] = "0"
+		mainThread.phpIni["max_input_time"] = "-1"
+	}
+
+	// Pass the php.ini overrides to PHP before startup
 	// TODO: if needed this would also be possible on a per-thread basis
-	overrides := ""
+	var overrides strings.Builder
 	for k, v := range mainThread.phpIni {
-		overrides += fmt.Sprintf("%s=%s\n", k, v)
+		overrides.WriteString(k)
+		overrides.WriteByte('=')
+		overrides.WriteString(v)
+		overrides.WriteByte('\n')
 	}
-	return C.CString(overrides)
+
+	return C.CString(overrides.String())
 }