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

PHP exit; fails to produce a CGI response #24

Open
fd opened this issue Jan 11, 2023 · 4 comments
Open

PHP exit; fails to produce a CGI response #24

fd opened this issue Jan 11, 2023 · 4 comments

Comments

@fd
Copy link

fd commented Jan 11, 2023

When exit; is called and the headers have not yet been flushed the CGI response is completely empty.
The issue seems to be that the shutdown procedures are not called on exit;.

Reproduction

<?php
// test-exit-flush-fail.php
header('X-Test: 123');
exit;
<?php
// test-exit-flush-ok.php
header('X-Test: 123');
// Note: at least one byte needs to be sent.
echo "X";
exit;

When I run the scripts with a native php-cgi binary it produces the expected CGI responses.

> docker run --rm -ti -v $PWD:/app php:7.4.32-cli /usr/local/bin/php-cgi /app/test-exit-flush-ok.php
X-Powered-By: PHP/7.4.32
X-Test: 123
Content-type: text/html; charset=UTF-8

X
> docker run --rm -ti -v $PWD:/app php:7.4.32-cli /usr/local/bin/php-cgi /app/test-exit-flush-fail.php
X-Powered-By: PHP/7.4.32
X-Test: 123
Content-type: text/html; charset=UTF-8

But when I run the test-exit-flush-fail.php with wasmtime it fails to produce a CGI response.

> wasmtime --mapdir=./::./ php-cgi -- test-exit-flush-ok.php 
X-Powered-By: PHP/7.4.32
X-Test: 123
Content-type: text/html; charset=UTF-8

X
> wasmtime --mapdir=./::./ php-cgi -- test-exit-flush-fail.php 
@fd
Copy link
Author

fd commented Jan 11, 2023

I did some investigating today. This bug is related to the lack of setjmp/longjmp support, obviously.

Ruby has a working version of the setjmp api. It relies on asyncify. I believe this approach was already mentioned in a blog post but not yet implemented.

I integrated the ruby code with php v7.4.32 and it seems to work well. I'd like to open a PR with the patch but I'm unsure how to produce the actual patch file from the build-staging directory.

@ereslibre
Copy link
Contributor

ereslibre commented Jan 12, 2023

Hello @fd! Thank you for taking the time to investigate this issue. You are absolutely right, this issue is due to the fact of a missing setjmp/longjmp emulation.

We are super interested in looking at your patch and integrating it!

When you clone the repo locally, you can add your patch to https://github.com/vmware-labs/webassembly-language-runtimes/tree/main/php/php-7.4.32/patches, and then from the root of the repo you should be able to build PHP by running make php/php-7.4.32.

You need docker or podman for this build to succeed. After it has been built, you can find the PHP artifact at php/build-output/php/php-7.4.32/bin.

@assambar
Copy link
Contributor

I integrated the ruby code with php v7.4.32 and it seems to work well. I'd like to open a PR with the patch but I'm unsure how to produce the actual patch file from the build-staging directory.

In case you did your changes in build-staging/php/php-7.4.32/checkout, here is what you can do to ensure the patch format is the same as the other patches.

  1. Commit your changes via pushd build-staging/php/php-7.4.32/checkout && git commit -m "..." && popd
  2. From the webassembly-language-runtimes repository root set up the build environment like source scripts/wl-env.sh php/php-7.4.32
  3. Update the patches via ./scripts/wl-update-patches.sh
  4. Cleanup the build environment via wl-env-unset
  5. Commit the updated patches and create a PR.

I'm looking forward to seeing what you did as we're working on enabling PHP 8 fibers via asyncify so this can be combined with your work to a better end.

@fd
Copy link
Author

fd commented Jan 13, 2023

I prepared the patch here #28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants