From 3dd8c91a2bb3cbb6d3da3f1e11f6ba9af47c6941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 10 Mar 2025 16:10:24 +0100 Subject: [PATCH 01/13] ci: run tests on macOS --- .github/workflows/tests.yaml | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 4efb0693af..ecc569091d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -71,3 +71,36 @@ jobs: if: matrix.php-versions == '8.4' with: version: latest + mac-tests: + runs-on: macos-latest + env: + GOEXPERIMENT: cgocheck2 + 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=$(php-config --includes)" >> "${GITHUB_ENV}" + - name: Build + run: go build -tags nowatcher + - name: Build testcli binary + working-directory: internal/testcli/ + 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 ./... From 50c261350f6574a3effea13edea20b8ed4e367b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 10 Mar 2025 23:05:11 +0100 Subject: [PATCH 02/13] debug --- .github/workflows/tests.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ecc569091d..d03ff4d77f 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -92,13 +92,11 @@ jobs: env: phpts: ts debug: true + - run: ls /usr/include/php - name: Set Set CGO flags - run: echo "CGO_CFLAGS=$(php-config --includes)" >> "${GITHUB_ENV}" + run: echo "CGO_CFLAGS=-I/usr/include/php $(php-config --includes)" >> "${GITHUB_ENV}" - name: Build run: go build -tags nowatcher - - name: Build testcli binary - working-directory: internal/testcli/ - run: go build -tags nowatcher - name: Run library tests run: go test -tags nowatcher -race -v ./... - name: Run Caddy module tests From 00289bfe83face24c4020e3e72f0f01843e4d950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 10:11:14 +0100 Subject: [PATCH 03/13] debug --- .github/workflows/tests.yaml | 2 +- CONTRIBUTING.md | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index d03ff4d77f..343c1ba8f4 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -92,9 +92,9 @@ jobs: env: phpts: ts debug: true - - run: ls /usr/include/php - name: Set Set CGO flags run: echo "CGO_CFLAGS=-I/usr/include/php $(php-config --includes)" >> "${GITHUB_ENV}" + - uses: mxschmitt/action-tmate@v3 - name: Build run: go build -tags nowatcher - name: Run library tests diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bf5c5ebdcd..b7b7796e66 100644 --- a/CONTRIBUTING.md +++ b/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 From 88ce8ef57d19b96b7a9a9868f9b060fb1efe7700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 10:51:18 +0100 Subject: [PATCH 04/13] fix --- .github/workflows/tests.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 343c1ba8f4..ff93f365ce 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -93,8 +93,11 @@ jobs: phpts: ts debug: true - name: Set Set CGO flags - run: echo "CGO_CFLAGS=-I/usr/include/php $(php-config --includes)" >> "${GITHUB_ENV}" - - uses: mxschmitt/action-tmate@v3 + run: | + { + echo "CGO_CFLAGS=$(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 From 77450d66914f8c039b9180741025c4bdcf6c7a6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 10:56:52 +0100 Subject: [PATCH 05/13] nobrotli --- .github/workflows/tests.yaml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ff93f365ce..51d315bcca 100644 --- a/.github/workflows/tests.yaml +++ b/.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,7 +72,8 @@ jobs: if: matrix.php-versions == '8.4' with: version: latest - mac-tests: + tests-mac: + name: Tests (macOS, PHP 8.4) runs-on: macos-latest env: GOEXPERIMENT: cgocheck2 @@ -104,4 +106,4 @@ jobs: 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 ./... + run: go test -tags nowatcher,nobrotli,nobadger,nomysql,nopgx -race -v ./... From 41da64203fe2f48d15d2c6e9a073b7727bf8e9fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 11:09:22 +0100 Subject: [PATCH 06/13] install brotli --- .github/workflows/tests.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 51d315bcca..e5f12cb7ea 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -77,6 +77,7 @@ jobs: runs-on: macos-latest env: GOEXPERIMENT: cgocheck2 + HOMEBREW_NO_AUTO_UPDATE: 1 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -94,6 +95,8 @@ jobs: env: phpts: ts debug: true + - name: Install Brotli + run: brew install brotli - name: Set Set CGO flags run: | { @@ -106,4 +109,4 @@ jobs: run: go test -tags nowatcher -race -v ./... - name: Run Caddy module tests working-directory: caddy/ - run: go test -tags nowatcher,nobrotli,nobadger,nomysql,nopgx -race -v ./... + run: go test -tags nowatcher,nobadger,nomysql,nopgx -race -v ./... From 67d4d723505a83f58d65272db8adb8d1d747f7b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 11:36:26 +0100 Subject: [PATCH 07/13] fix --- .github/workflows/tests.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index e5f12cb7ea..fcc73b564b 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -95,12 +95,10 @@ jobs: env: phpts: ts debug: true - - name: Install Brotli - run: brew install brotli - name: Set Set CGO flags run: | { - echo "CGO_CFLAGS=$(php-config --includes)" + echo "CGO_CFLAGS=-I/opt/homebrew/include/brotli/d $(php-config --includes)" echo "CGO_LDFLAGS=-L/opt/homebrew/lib/ $(php-config --ldflags) $(php-config --libs)" } >> "${GITHUB_ENV}" - name: Build From 08818afe1f0df557adf2d42f922308c8da608ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 11:44:33 +0100 Subject: [PATCH 08/13] fix --- .github/workflows/tests.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index fcc73b564b..f4ae6b8832 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -98,7 +98,7 @@ jobs: - name: Set Set CGO flags run: | { - echo "CGO_CFLAGS=-I/opt/homebrew/include/brotli/d $(php-config --includes)" + 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 From 1775ca55f23b598487bf3b4ce1bd18f1053e03fd Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Tue, 11 Mar 2025 15:25:16 +0100 Subject: [PATCH 09/13] Also registers php.ini if ZEND_MAX_EXECUTION_TIMERS is disabled. --- frankenphp.c | 26 ++++++-------------------- phpmainthread.go | 10 ++++++++-- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index c4af6c2afc..6d35fc3b70 100644 --- a/frankenphp.c +++ b/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() { @@ -901,24 +894,17 @@ 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; -#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)); -#endif + /* overwrite php.ini with custom user settings and disable + * max_execution_timers */ + char *php_ini_overrides = go_get_custom_php_ini(true); #else /* overwrite php.ini with custom user settings */ - char *php_ini_overrides = go_get_custom_php_ini(); + char *php_ini_overrides = go_get_custom_php_ini(false); +#endif + if (php_ini_overrides != NULL) { frankenphp_sapi_module.ini_entries = php_ini_overrides; } -#endif frankenphp_sapi_module.startup(&frankenphp_sapi_module); diff --git a/phpmainthread.go b/phpmainthread.go index 559c589641..2234ee2666 100644 --- a/phpmainthread.go +++ b/phpmainthread.go @@ -191,9 +191,15 @@ 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(disableExecutionTimers C.bool) *C.char { if mainThread.phpIni == nil { - return nil + mainThread.phpIni = make(map[string]string) + } + + // hardcoded ini settings if ZEND_MAX_EXECUTION_TIMERS is disabled + if disableExecutionTimers { + mainThread.phpIni["max_execution_time"] = "0" + mainThread.phpIni["max_input_time"] = "-1" } // pass the php.ini overrides to PHP before startup From b05bd5feae8930b8fd03dd75a437d3fe4ee30e90 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Tue, 11 Mar 2025 15:35:07 +0100 Subject: [PATCH 10/13] Removes max_execution_time from tests (it gets overwritten on mac) --- caddy/caddy_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 0d374f47fc..2ff8bf207b 100644 --- a/caddy/caddy_test.go +++ b/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") } From bcf289b11d14eee604fc929e3ad3e52576ce3491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 16:27:05 +0100 Subject: [PATCH 11/13] tiny refacto --- frankenphp.c | 8 ++++---- phpmainthread.go | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index 6d35fc3b70..abf5feb778 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -893,13 +893,13 @@ static void *php_main(void *arg) { sapi_startup(&frankenphp_sapi_module); -#ifndef ZEND_MAX_EXECUTION_TIMERS +#ifdef ZEND_MAX_EXECUTION_TIMERS + /* overwrite php.ini with custom user settings */ + char *php_ini_overrides = go_get_custom_php_ini(false); +#else /* overwrite php.ini with custom user settings and disable * max_execution_timers */ char *php_ini_overrides = go_get_custom_php_ini(true); -#else - /* overwrite php.ini with custom user settings */ - char *php_ini_overrides = go_get_custom_php_ini(false); #endif if (php_ini_overrides != NULL) { diff --git a/phpmainthread.go b/phpmainthread.go index 2234ee2666..c61e4fdf79 100644 --- a/phpmainthread.go +++ b/phpmainthread.go @@ -196,13 +196,15 @@ func go_get_custom_php_ini(disableExecutionTimers C.bool) *C.char { mainThread.phpIni = make(map[string]string) } - // hardcoded ini settings if ZEND_MAX_EXECUTION_TIMERS is disabled + // 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 disableExecutionTimers { mainThread.phpIni["max_execution_time"] = "0" mainThread.phpIni["max_input_time"] = "-1" } - // pass the php.ini overrides to PHP before startup + // Pass the php.ini overrides to PHP before startup // TODO: if needed this would also be possible on a per-thread basis overrides := "" for k, v := range mainThread.phpIni { From dc138e01d163b02387efe67ad1416b7dbf9923e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 17:07:42 +0100 Subject: [PATCH 12/13] fix free --- frankenphp.c | 6 +++--- phpmainthread.go | 16 ++++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index abf5feb778..740be18b46 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -924,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; } diff --git a/phpmainthread.go b/phpmainthread.go index c61e4fdf79..7245db3716 100644 --- a/phpmainthread.go +++ b/phpmainthread.go @@ -8,7 +8,7 @@ package frankenphp // #include "frankenphp.h" import "C" import ( - "fmt" + "strings" "sync" "github.com/dunglas/frankenphp/internal/memory" @@ -191,7 +191,7 @@ func go_frankenphp_shutdown_main_thread() { } //export go_get_custom_php_ini -func go_get_custom_php_ini(disableExecutionTimers C.bool) *C.char { +func go_get_custom_php_ini(disableTimeouts C.bool) *C.char { if mainThread.phpIni == nil { mainThread.phpIni = make(map[string]string) } @@ -199,16 +199,20 @@ func go_get_custom_php_ini(disableExecutionTimers C.bool) *C.char { // 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 disableExecutionTimers { + 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()) } From f74846d383035bcc62e0b8b6d23ca68942be3603 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 11 Mar 2025 17:17:42 +0100 Subject: [PATCH 13/13] cs --- frankenphp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frankenphp.c b/frankenphp.c index 740be18b46..a20d40186d 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -925,7 +925,7 @@ static void *php_main(void *arg) { #endif if (frankenphp_sapi_module.ini_entries) { - free((char *) frankenphp_sapi_module.ini_entries); + free((char *)frankenphp_sapi_module.ini_entries); frankenphp_sapi_module.ini_entries = NULL; }