Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: signals handling #39

Merged
merged 1 commit into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 23 additions & 30 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,64 +1,57 @@
# Contributing

## Running the test suite

go test -race -v ./...

## Debugging
## Compiling PHP
### 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
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...).
#### Caddy module

Build Caddy with the FrankenPHP Caddy module:
### Without Docker (Linux and macOS)

cd /go/src/app/caddy/frankenphp/
go build
[Follow the instructions to compile from sources](docs/compile.md) and pass the `--debug` configuration flag.

Run the Caddy with the FrankenPHP Caddy module:
## Running the test suite

cd /go/src/app/testdata/
../caddy/frankenphp/frankenphp run
go test -race -v ./...

#### Minimal test server
## Caddy module

Build the minimal test server:
Build Caddy with the FrankenPHP Caddy module:

cd /go/src/app/internal/testserver/
cd caddy/frankenphp/
go build
cd ../../

Run the test server:
Run the Caddy with the FrankenPHP Caddy module:

cd /go/src/app/testdata/
../internal/testserver/testserver
cd testdata/
../caddy/frankenphp/frankenphp run

The server is listening on `127.0.0.1:8080`:

curl http://127.0.0.1:8080/phpinfo.php

### Without Docker (Linux and macOS)

Compile PHP:
curl -vk https://localhosy/phpinfo.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)
Expand Down
3 changes: 1 addition & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,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 && \
make -j$(nproc) && \
Expand Down
9 changes: 5 additions & 4 deletions Dockerfile.dev
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -59,3 +58,5 @@ WORKDIR /go/src/app
COPY . .

RUN go get -d -v ./...

CMD [ "zsh" ]
3 changes: 1 addition & 2 deletions docs/compile.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -58,7 +57,7 @@ if needed.
Finally, compile PHP:

```
make -j6
make -j$(nproc)
make install
```

Expand Down
38 changes: 28 additions & 10 deletions frankenphp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -32,6 +37,10 @@ int frankenphp_check_version() {
return -2;
}

#ifdef ZEND_SIGNALS
return -3;
#endif

return SUCCESS;
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand All @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion frankenphp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
dunglas marked this conversation as resolved.
Show resolved Hide resolved
MainThreadCreationError = errors.New("error creating the main thread")
RequestContextCreationError = errors.New("error during request context creation")
RequestStartupError = errors.New("error during PHP request startup")
Expand Down Expand Up @@ -203,6 +204,9 @@ func Init(options ...Option) error {

case -2:
return InvalidPHPVersionError

case -3:
return ZendSignalsError
}

shutdownWG.Add(1)
Expand Down