From 1e453036b2f0f457718ea7c4d18737ced84b5c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Wed, 19 Oct 2022 11:19:49 +0200 Subject: [PATCH] fix: signals handling (#39) --- CONTRIBUTING.md | 61 ++++++++++++++++++++++--------------------------- Dockerfile | 3 +-- Dockerfile.dev | 9 ++++---- docs/compile.md | 3 +-- frankenphp.c | 38 ++++++++++++++++++++++-------- frankenphp.go | 6 ++++- 6 files changed, 67 insertions(+), 53 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2465f2e..00e93c3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,64 +1,57 @@ # Contributing +## Compiling PHP +### With Docker (Linux) + +Build the dev Docker image: + + docker -t frankenphp-dev -f Dockerfile.dev . + docker run -p 8080:8080 -p 443:443 -v $PWD:/go/src/app -it frankenphp-dev + +The image contains the usual development tools (Go, GDB, Valgrind, Neovim...). + +### Without Docker (Linux and macOS) + +[Follow the instructions to compile from sources](docs/compile.md) and pass the `--debug` configuration flag. + ## Running the test suite go test -race -v ./... -## Debugging -### With Docker (Linux) - -Build the dev Docker image: - - docker build -t frankenphp-dev Dockerfile.dev - docker run -p 8080:8080 -p 443:443 -v $PWD:/go/src/app -it frankenphp-dev bash - -The image contains the usual development tools (Go, GDB, Valgrind, Neovim...). -#### Caddy module +## Caddy module Build Caddy with the FrankenPHP Caddy module: - cd /go/src/app/caddy/frankenphp/ + cd caddy/frankenphp/ go build + cd ../../ Run the Caddy with the FrankenPHP Caddy module: - cd /go/src/app/testdata/ + cd testdata/ ../caddy/frankenphp/frankenphp run -#### Minimal test server - -Build the minimal test server: - - cd /go/src/app/internal/testserver/ - go build - -Run the test server: - - cd /go/src/app/testdata/ - ../internal/testserver/testserver - The server is listening on `127.0.0.1:8080`: - curl http://127.0.0.1:8080/phpinfo.php + curl -vk https://localhosy/phpinfo.php -### Without Docker (Linux and macOS) - -Compile PHP: - - ./configure --enable-debug --enable-zts - make -j6 - sudo make install +## Minimal test server Build the minimal test server: cd internal/testserver/ go build + cd ../../ -Run the test app: +Run the test server: - cd ../../testdata/ + cd testdata/ ../internal/testserver/testserver +The server is listening on `127.0.0.1:8080`: + + curl -v http://127.0.0.1:8080/phpinfo.php + ## Misc Dev Resources * [PHP embedding in uWSGI](https://github.com/unbit/uwsgi/blob/master/plugins/php/php_plugin.c) diff --git a/Dockerfile b/Dockerfile index 22732b0..39e3df7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,11 +34,10 @@ RUN apt-get update && \ RUN git clone --depth=1 --single-branch --branch=frankenphp-8.2 https://github.com/dunglas/php-src.git && \ cd php-src && \ - #export CFLAGS="-DNO_SIGPROF" && \ # --enable-embed is only necessary to generate libphp.so, we don't use this SAPI directly ./buildconf && \ ./configure \ - --enable-embed=static \ + --enable-embed \ --enable-zts \ --disable-zend-signals \ # --enable-mysqlnd is included here because it's harder to compile after the fact than extensions are (since it's a plugin for several extensions, not an extension in itself) diff --git a/Dockerfile.dev b/Dockerfile.dev index 79955d2..fa77655 100644 --- a/Dockerfile.dev +++ b/Dockerfile.dev @@ -36,18 +36,17 @@ RUN apt-get update && \ && \ apt-get clean -RUN git clone --depth=1 --single-branch --branch=frankenphp-8.2 https://github.com/dunglas/php-src.git && \ +RUN git clone --depth=1 https://github.com/php/php-src.git && \ cd php-src && \ git checkout frankenphp-8.2 && \ - export CFLAGS="-DNO_SIGPROF" && \ # --enable-embed is only necessary to generate libphp.so, we don't use this SAPI directly ./buildconf && \ ./configure \ - --enable-embed=static \ + --enable-embed \ --enable-zts \ --disable-zend-signals \ --enable-debug && \ - make -j6 && \ + make -j$(nproc) && \ make install && \ ldconfig && \ php --version @@ -59,3 +58,5 @@ WORKDIR /go/src/app COPY . . RUN go get -d -v ./... + +CMD [ "zsh" ] diff --git a/docs/compile.md b/docs/compile.md index 9e4cb24..36ef10c 100644 --- a/docs/compile.md +++ b/docs/compile.md @@ -39,7 +39,6 @@ echo 'export PATH="/opt/homebrew/opt/bison/bin:$PATH"' >> ~/.zshrc Then run the configure script: ``` -export CFLAGS="-DNO_SIGPROF" ./configure \ --enable-embed=static \ --enable-zts \ @@ -58,7 +57,7 @@ if needed. Finally, compile PHP: ``` -make -j6 +make -j$(nproc) make install ``` diff --git a/frankenphp.c b/frankenphp.c index 860e0ba..fea0103 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -23,6 +23,11 @@ ZEND_TSRMLS_CACHE_DEFINE() #endif +/* Timeouts are currently fundamentally broken with ZTS: https://bugs.php.net/bug.php?id=79464 */ +const char HARDCODED_INI[] = + "max_execution_time=0\n" + "max_input_time=-1\n\0"; + int frankenphp_check_version() { #ifndef ZTS return -1; @@ -32,6 +37,10 @@ int frankenphp_check_version() { return -2; } +#ifdef ZEND_SIGNALS + return -3; +#endif + return SUCCESS; } @@ -50,9 +59,9 @@ static void frankenphp_worker_request_shutdown(uintptr_t current_request) { } zend_end_try(); /* Reset max_execution_time (no longer executing php code after response sent) */ - zend_try { + /*zend_try { zend_unset_timeout(); - } zend_end_try(); + } zend_end_try();*/ /* Shutdown output layer (send the set HTTP headers, cleanup output handlers, etc.) */ zend_try { @@ -84,11 +93,15 @@ static int frankenphp_worker_request_startup() { /* Keep the current execution context */ sapi_activate(); - if (PG(max_input_time) == -1) { - zend_set_timeout(EG(timeout_seconds), 1); - } else { - zend_set_timeout(PG(max_input_time), 1); - } + /* + * Timeouts are currently fundamentally broken with ZTS: https://bugs.php.net/bug.php?id=79464 + * + *if (PG(max_input_time) == -1) { + * zend_set_timeout(EG(timeout_seconds), 1); + *} else { + * zend_set_timeout(PG(max_input_time), 1); + *} + */ if (PG(expose_php)) { sapi_add_header(SAPI_PHP_VERSION_HEADER, sizeof(SAPI_PHP_VERSION_HEADER)-1, 1); @@ -449,11 +462,11 @@ static void *manager_thread(void *arg) { # endif #endif -#ifdef ZEND_SIGNALS - zend_signal_startup(); -#endif sapi_startup(&frankenphp_sapi_module); + frankenphp_sapi_module.ini_entries = malloc(sizeof(HARDCODED_INI)); + memcpy(frankenphp_sapi_module.ini_entries, HARDCODED_INI, sizeof(HARDCODED_INI)); + frankenphp_sapi_module.startup(&frankenphp_sapi_module); threadpool thpool = thpool_init(*((int *) arg)); @@ -475,6 +488,11 @@ static void *manager_thread(void *arg) { tsrm_shutdown(); #endif + if (frankenphp_sapi_module.ini_entries) { + free(frankenphp_sapi_module.ini_entries); + frankenphp_sapi_module.ini_entries = NULL; + } + go_shutdown(); return NULL; diff --git a/frankenphp.go b/frankenphp.go index 0c1a8a6..4f7acc0 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -5,7 +5,7 @@ // [FrankenPHP app server]: https://frankenphp.dev package frankenphp -// #cgo CFLAGS: -DNO_SIGPROF -Wall +// #cgo CFLAGS: -Wall // #cgo CFLAGS: -I/usr/local/include/php -I/usr/local/include/php/Zend -I/usr/local/include/php/TSRM -I/usr/local/include/php/main // #cgo LDFLAGS: -L/usr/local/lib -L/opt/homebrew/opt/libiconv/lib -L/usr/lib -lphp -lxml2 -lresolv -lsqlite3 -ldl -lm -lutil // #cgo darwin LDFLAGS: -liconv @@ -41,6 +41,7 @@ var ( InvalidRequestError = errors.New("not a FrankenPHP request") AlreaydStartedError = errors.New("FrankenPHP is already started") InvalidPHPVersionError = errors.New("FrankenPHP is only compatible with PHP 8.2+") + ZendSignalsError = errors.New("Zend Signals are enabled, recompible PHP with --disable-zend-signals") MainThreadCreationError = errors.New("error creating the main thread") RequestContextCreationError = errors.New("error during request context creation") RequestStartupError = errors.New("error during PHP request startup") @@ -203,6 +204,9 @@ func Init(options ...Option) error { case -2: return InvalidPHPVersionError + + case -3: + return ZendSignalsError } shutdownWG.Add(1)